remove z_travel from tecan resources #553

rickwierenga · 2025-06-05T03:37:20Z

see #552 for background

rickwierenga · 2025-06-05T03:58:50Z

i'm afraid the previous implementation was even wrong since the travel height only considered the specific resource in question but not the other resources around

in the future we can write a nice and optimal algorithm that computes the minimum height required for the marginal speed benefit, hopefully on the LH level, but for now just increase it to a safe height that should work everywhere.

rickwierenga · 2025-06-05T04:24:29Z

updated the evo backend with f4705de

note that the tests don't pass because this call changed:

call(module='C5', command='SHZ', params=[1455, 1455, 1455, 1455, 1455, 1455, 1455, 1455]),
call(module='C5', command='MDT', params=[1, None, None, None, 30, 0, 0, 0, 0, 0, 0, 0]),
call(module='C5', command='SHZ', params=[2000, 2000, 2000, 2000, 2000, 2000, 2000, 2000]),

to this

call(module='C5', command='SHZ', params=[945, 945, 945, 945, 945, 945, 945, 945]),
call(module='C5', command='MDT', params=[1, None, None, None, 30, 0, 0, 0, 0, 0, 0, 0]),
call(module='C5', command='SHZ', params=[2000, 2000, 2000, 2000, 2000, 2000, 2000, 2000]),

i'll need someone to test this on an EVO before merging to make sure it doesn't break anything. It seems to me it wouldn't because the z travel height is not important right before MDT ("Move Tip, Detect Liquid, Submerge", move_detect_liquid), but good to confirm.

nedru004 · 2025-06-11T14:17:32Z

This PR will be very helpful. I have crashed my arm multiple times now since the z_travel is set for the destination resource and not anything in the way.

I tried running the unittest for the tecan backend but I am not very experienced with this. Do I need to have the resources listed or should I change them to match my set up? I also removed the RoMa arm since it was getting in the way and not needed at the moment.

I get this error when running the test.

/usr/local/bin/python3.12 -m unittest pylabrobot/liquid_handling/backends/tecan/EVO_tests.py
F.F.
======================================================================
FAIL: test_aspirate (pylabrobot.liquid_handling.backends.tecan.EVO_tests.EVOTests.test_aspirate)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/unittest/async_case.py", line 90, in _callTestMethod
    if self._callMaybeAsync(method) is not None:
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/unittest/async_case.py", line 112, in _callMaybeAsync
    return self._asyncioRunner.run(
           ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/asyncio/runners.py", line 118, in run
    return self._loop.run_until_complete(task)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/asyncio/base_events.py", line 685, in run_until_complete
    return future.result()
           ^^^^^^^^^^^^^^^
  File "/Users/david.nedrud/Documents/GitHub/pylabrobot/pylabrobot/liquid_handling/backends/tecan/EVO_tests.py", line 148, in test_aspirate
    self.evo.send_command.assert_has_calls(  # type: ignore[attr-defined]
  File "/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/unittest/mock.py", line 981, in assert_has_calls
    raise AssertionError(
AssertionError: Calls not found.
Expected: [call(module='C5', command='SHZ', params=[2000, 2000, 2000, 2000, 2000, 2000, 2000, 2000]),
 call(module='C5', command='RPX', params=[0]),
 call(module='C5', command='PAA', params=[3829, 2051, 90, 1455, 2000, 2000, 2000, 2000, 2000, 2000, 2000]),
 call(module='C5', command='PVL', params=[0, None, None, None, None, None, None, None]),
 call(module='C5', command='SEP', params=[840, None, None, None, None, None, None, None]),
 call(module='C5', command='PPR', params=[30, None, None, None, None, None, None, None]),
 call(module='C5', command='SDM', params=[7, 1]),
 call(module='C5', command='SSL', params=[600, None, None, None, None, None, None, None]),
 call(module='C5', command='SDL', params=[40, None, None, None, None, None, None, None]),
 call(module='C5', command='STL', params=[1375, None, None, None, None, None, None, None]),
 call(module='C5', command='SML', params=[985, 2000, 2000, 2000, 2000, 2000, 2000, 2000]),
 call(module='C5', command='SBL', params=[20, None, None, None, None, None, None, None]),
 call(module='C5', command='SHZ', params=[1455, 1455, 1455, 1455, 1455, 1455, 1455, 1455]),
 call(module='C5', command='MDT', params=[1, None, None, None, 30, 0, 0, 0, 0, 0, 0, 0]),
 call(module='C5', command='SHZ', params=[2000, 2000, 2000, 2000, 2000, 2000, 2000, 2000]),
 call(module='C5', command='SSZ', params=[30, None, None, None, None, None, None, None]),
 call(module='C5', command='SEP', params=[1200, None, None, None, None, None, None, None]),
 call(module='C5', command='STZ', params=[-30, None, None, None, None, None, None, None]),
 call(module='C5', command='MTR', params=[626, None, None, None, None, None, None, None]),
 call(module='C5', command='SSZ', params=[200, None, None, None, None, None, None, None]),
 call(module='C5', command='MAZ', params=[1375, None, None, None, None, None, None, None]),
 call(module='C5', command='PVL', params=[0, None, None, None, None, None, None, None]),
 call(module='C5', command='SEP', params=[840, None, None, None, None, None, None, None]),
 call(module='C5', command='PPR', params=[60, None, None, None, None, None, None, None])]
  Actual: [call(module='C5', command='SHZ', params=[2000, 2000, 2000, 2000, 2000, 2000, 2000, 2000]),
 call(module='C5', command='RPX', params=[0]),
 call(module='C5', command='PAA', params=[3829, 2051, 90, 945, 2000, 2000, 2000, 2000, 2000, 2000, 2000]),
 call(module='C5', command='PVL', params=[0, None, None, None, None, None, None, None]),
 call(module='C5', command='SEP', params=[840, None, None, None, None, None, None, None]),
 call(module='C5', command='PPR', params=[30, None, None, None, None, None, None, None]),
 call(module='C5', command='SDM', params=[7, 1]),
 call(module='C5', command='SSL', params=[600, None, None, None, None, None, None, None]),
 call(module='C5', command='SDL', params=[40, None, None, None, None, None, None, None]),
 call(module='C5', command='STL', params=[1375, None, None, None, None, None, None, None]),
 call(module='C5', command='SML', params=[985, 2000, 2000, 2000, 2000, 2000, 2000, 2000]),
 call(module='C5', command='SBL', params=[20, None, None, None, None, None, None, None]),
 call(module='C5', command='SHZ', params=[945, 945, 945, 945, 945, 945, 945, 945]),
 call(module='C5', command='MDT', params=[1, None, None, None, 30, 0, 0, 0, 0, 0, 0, 0]),
 call(module='C5', command='SHZ', params=[2000, 2000, 2000, 2000, 2000, 2000, 2000, 2000]),
 call(module='C5', command='SSZ', params=[30, None, None, None, None, None, None, None]),
 call(module='C5', command='SEP', params=[1200, None, None, None, None, None, None, None]),
 call(module='C5', command='STZ', params=[-30, None, None, None, None, None, None, None]),
 call(module='C5', command='MTR', params=[626, None, None, None, None, None, None, None]),
 call(module='C5', command='SSZ', params=[200, None, None, None, None, None, None, None]),
 call(module='C5', command='MAZ', params=[1375, None, None, None, None, None, None, None]),
 call(module='C5', command='PVL', params=[0, None, None, None, None, None, None, None]),
 call(module='C5', command='SEP', params=[840, None, None, None, None, None, None, None]),
 call(module='C5', command='PPR', params=[60, None, None, None, None, None, None, None])]

rickwierenga · 2025-06-11T18:16:51Z

I tried running the unittest for the tecan backend but I am not very experienced with this. Do I need to have the resources listed or should I change them to match my set up? I also removed the RoMa arm since it was getting in the way and not needed at the moment.

The unit tests are actually completely independent of hardware (in fact, they even run in github ci!)

the problem is we hardcode the firmware commands that we expect to see, and then see if the calls generate them. the thought is that if we generate and send the same firmware commands, the machines will do the same thing. PLR's responsibility ends at generating and sending these commands, so testing here makes sure we do what we need to do.

The reason I raised it here is I never change the tests without verifying on a physical machine. And since I currently don't have an EVO, I was blocked on that for this PR.

But now that you have tested (thank you!) I will update the tests to reflect this new behavior and merge it.

nedru004 · 2025-06-11T20:35:19Z

Ok. I tested the changes to z_travel and found that it traveled lower than before. It still uses the resource z size to determine the travel height.

     # z travel seems to only be used for aspiration and dispense right now
      if isinstance(op, (SingleChannelAspiration, SingleChannelDispense)):
        z_positions["travel"][channel] = get_z_position(
          self._z_traversal_height * 10, par.get_absolute_location().z + op.offset.z, tip_length
        )

I changed it to just use the _z_traversal_height

     # z travel seems to only be used for aspiration and dispense right now
      if isinstance(op, (SingleChannelAspiration, SingleChannelDispense)):
        z_positions["travel"][channel] = get_z_position(self._z_traversal_height * 10)

That seemed to force the machine to traverse at the highest z. Do we need to change the "travel" height every time we perform an action? I think it is stored in the machine until we change it

rickwierenga · 2025-06-11T21:07:31Z

get_z_position(self._z_traversal_height * 10)

I don't understand how this works since get_z_position requires three arguments, and is apparently dependent on tip channel length. (Again, I can't test this locally unfortunately.)

Can you please verify the exact code that works? Ideally, could you create a PR into this branch https://github.com/PyLabRobot/pylabrobot/tree/remove-tecan-z-travel with your changes that work? Then I can merge this into main.

Do we need to change the "travel" height every time we perform an action? I think it is stored in the machine until we change it

You are right, we do not have to send the command (SHZ) every time. In a future update, we can cache the location in the set_z_travel_height method. (And other EVO backend methods.)

But the current issue is computing this location correctly and verifying it works. Caching should definitely not change that! Caching would just mean less commands to send but keep the behavior the same. The current behavior is wrong so we need to fix it first.

nedru004 · 2025-06-11T21:53:00Z

Ahh. Sorry I copied it wrong. I will create a PR with my working code. It should be this:

     # z travel seems to only be used for aspiration and dispense right now
      if isinstance(op, (SingleChannelAspiration, SingleChannelDispense)):
        z_positions["travel"][channel] = self._z_traversal_height * 10

rickwierenga · 2025-06-11T21:56:05Z

thanks. so not dependent on tip height?

nedru004 · 2025-06-12T18:46:31Z

Is z_travel the height from the deck? In the function get_z_position(), the z_range (which is 2100) - z (in this case z_travel) + z_off*10 + tip_length.

    def get_z_position(z, z_off, tip_length):
      # TODO: simplify z units
      return int(self._z_range - z + z_off * 10 + tip_length)  # TODO: verify z formula

Why is z_travel subtracted from z_range? If we set z_travel to 2100 then the height becomes 0 + the resource and tip_length.

Can you help me understand the values? I pulled these parameters from a tecan 96 well plate.

    z_travel=1857.0,
    z_start=1900.0,
    z_dispense=1915.0,
    z_max=2095.0,

Why is z_travel less than the variables? Are they an offset? In the function get_z_position() they subtract from the range.

Side note: In the future, will you be using z_travel to set the traverse height or manually move the arm to a specified height and then traverse?

rickwierenga · 2025-06-12T18:54:37Z

Those are all good questions, and I should have asked the person who wrote this backend about this in more detail when he contributed it.

It is very possible these values/logic are actually incorrect, so we should just be reasoning from first principles, and what works in your setup.

If we set z_travel to 2100 then the height becomes 0 + the resource and tip_length.

with this change (that you posted here), that should no longer be the case right?

to me it makes sense to "hardcode" (as an attribute of the backend) the traversal height and always use this value.

As you point out, they seem to be treated as offsets but are almost certainly not offsets when you look at their values. let's fix that.

The only thing that could be different is if it depends on tip height.

Why is z_travel subtracted from z_range?

Possibly not parsed correctly from the resource definition?

Either way, we don't care because this PR removes that attribute.

Can you help me understand the values? I pulled these parameters from a tecan 96 well plate.

See this: https://www.tecan.com/knowledge-portal/how-to-teach-the-z-values-in-freedom-evoware for explanation

We should get rid of those parameters in favor of general use parameters as explained in #552. On Hamiltons and OT, these parameters don't exist. First principles say we shouldn't need them.

nedru004 · 2025-06-13T19:24:52Z

Thanks so much for the detailed response. I will work on simplifying the coordinates and getting rid of tecan specific parameters. With this PR, the arm now retracts to full height before traversing. I am working on removing the other tecan specific parameters in another branch.

To stay consistent, the code should force "travel" to be an integer. Doesnt seem to affect the execution though.

     # z travel seems to only be used for aspiration and dispense right now
     if isinstance(op, (SingleChannelAspiration, SingleChannelDispense)):
        z_positions["travel"][channel] = int(self._z_traversal_height * 10)

rickwierenga · 2025-06-14T01:46:16Z

I will work on simplifying the coordinates and getting rid of tecan specific parameters.

Great! Happy to help with this.

To stay consistent, the code should force "travel" to be an integer. Doesnt seem to affect the execution though.

Since it should be in PLR units (mm, uL, s, mg, etc.), I think it should be float. The firmware accepts parameters in 0.1mm level, so we should just round and send when converting to firmware commands.

rickwierenga · 2025-06-16T20:07:53Z

thanks @nedru004 for your help with this!

remove z_travel from tecan resources

f608961

update evo backend

f4705de

rickwierenga marked this pull request as draft June 8, 2025 17:49

rickwierenga added 2 commits June 11, 2025 15:57

fix z travel & tests

d43cbf5

format

5be4e02

rickwierenga marked this pull request as ready for review June 12, 2025 17:30

nedru004 mentioned this pull request Jun 13, 2025

Remove Tecan-Specific Attributes from resource model #552

Open

12 tasks

rickwierenga added 2 commits June 16, 2025 11:54

Merge branch 'main' into remove-tecan-z-travel

2a5c33b

round z travel

80aa07f

rickwierenga merged commit 77843d9 into main Jun 16, 2025
6 checks passed

rickwierenga deleted the remove-tecan-z-travel branch June 16, 2025 20:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

remove z_travel from tecan resources #553

remove z_travel from tecan resources #553

Uh oh!

rickwierenga commented Jun 5, 2025

Uh oh!

rickwierenga commented Jun 5, 2025

Uh oh!

rickwierenga commented Jun 5, 2025

Uh oh!

nedru004 commented Jun 11, 2025 •

edited

Loading

Uh oh!

rickwierenga commented Jun 11, 2025

Uh oh!

nedru004 commented Jun 11, 2025

Uh oh!

rickwierenga commented Jun 11, 2025

Uh oh!

nedru004 commented Jun 11, 2025

Uh oh!

rickwierenga commented Jun 11, 2025

Uh oh!

nedru004 commented Jun 12, 2025

Uh oh!

rickwierenga commented Jun 12, 2025

Uh oh!

nedru004 commented Jun 13, 2025

Uh oh!

rickwierenga commented Jun 14, 2025

Uh oh!

Uh oh!

rickwierenga commented Jun 16, 2025

Uh oh!

Uh oh!

remove z_travel from tecan resources #553

remove z_travel from tecan resources #553

Uh oh!

Conversation

rickwierenga commented Jun 5, 2025

Uh oh!

rickwierenga commented Jun 5, 2025

Uh oh!

rickwierenga commented Jun 5, 2025

Uh oh!

nedru004 commented Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rickwierenga commented Jun 11, 2025

Uh oh!

nedru004 commented Jun 11, 2025

Uh oh!

rickwierenga commented Jun 11, 2025

Uh oh!

nedru004 commented Jun 11, 2025

Uh oh!

rickwierenga commented Jun 11, 2025

Uh oh!

nedru004 commented Jun 12, 2025

Uh oh!

rickwierenga commented Jun 12, 2025

Uh oh!

nedru004 commented Jun 13, 2025

Uh oh!

rickwierenga commented Jun 14, 2025

Uh oh!

Uh oh!

rickwierenga commented Jun 16, 2025

Uh oh!

Uh oh!

nedru004 commented Jun 11, 2025 •

edited

Loading