Nimbus v1 tcp aligned by cmoscy · Pull Request #999 · PyLabRobot/pylabrobot

cmoscy · 2026-04-18T05:59:08Z

Mostly @rickwierenga's nimbus v1 port. Aligned with updated tcp_client, introspection, and enriched HoiError+Channelized error driver.

… HOI mapping

- NimbusDriver on HamiltonTCPClient with NIMBUS_ERROR_CODES; chatterbox, door, pip backend - TCPCommand as the shared wire command base; client/introspection aligned - Nimbus unit tests for error-code merge and interface resolution

…Nimbus errors & status exceptions

Move HOI error parsing (parse_hamilton_error_*) from messages into hoi_error. Pass HOI entries and per-entry exceptions into ChannelizedError for callers.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

rickwierenga · 2026-04-18T20:06:52Z

One big thing this PR adds is liquid classes to the Nimbus, which hasn't existed in v0. I actually do not like liquid classes, because they are redundant with the parameters that you can already pass through standard python (whether directly through the capability or through backend params). A "liquid class" is just a 'set of parameters'. It's a relic from venus and anti pythonic imo, so long term I hope to get rid of them, in favor of just storing these 'sets of parameters' in NimbusPIPAspirateParams (or nested NimbusPIPBackend.AspirationParams to be more closely aligned with the star and other backends etc.)

Like you can just store it as follows

liquid_class = NimbusPIPBackend.AspirationParams(
  x=1,
  y=2,
)

this is essentially equivalent to a "liquid class" concept in lab automation, and removes the ambiguity (with separate an hlc parameter, you have three sources of information: explicitly passed, hlc, and backend default. not good)

would that work for you?

cmoscy · 2026-04-18T20:12:36Z

One big thing this PR adds is liquid classes to the Nimbus, which hasn't existed in v0. I actually do not like liquid classes, because they are redundant with the parameters that you can already pass through standard python (whether directly through the capability or through backend params). A "liquid class" is just a 'set of parameters'. It's a relic from venus and anti pythonic imo, so long term I hope to get rid of them, in favor of just storing these 'sets of parameters' in NimbusPIPAspirateParams (or nested NimbusPIPBackend.AspirationParams to be more closely aligned with the star and other backends etc.)

Like you can just store it as follows
liquid_class = NimbusPIPBackend.AspirationParams(
  x=1,
  y=2,
)
this is essentially equivalent to a "liquid class" concept in lab automation, and removes the ambiguity (with separate an hlc parameter, you have three sources of information: explicitly passed, hlc, and backend default. not good)

would that work for you?

That works for me. Agree that could be a lot better and direct. Does this also include the volume correction curves that come along? Since some of those defaults can be useful as a starting point/reference.

Want me to pull the liquid_class wiring from nimbus and prep for now? Mainly had it just for consistency with star (+vol corrections)

rickwierenga · 2026-04-18T20:16:31Z

that is an unsolved question :) open to suggestions. I think passing a function/calibration curve would be neat, or if no function/curve is passed just use the value directly, with the default being the current default hlc curve.

I am saying it more from a "long term I want to get rid of liquid classes and figure out a better way" point of view, so would prefer to not add it in new code and rather focus on getting it out of old code if that makes sense.

cmoscy · 2026-04-18T20:18:42Z

that is an unsolved question :) open to suggestions. I think passing a function/calibration curve would be neat, or if no function/curve is passed just use the value directly, with the default being the current default hlc curve.

I am saying it more from a "long term I want to get rid of liquid classes and figure out a better way" point of view, so would prefer to not add it in new code and rather focus on getting it out of old code if that makes sense.

Totally makes sense. Maybe I can play around a bit and see if there's any solutions there. If I get any ideas I'll put them up on the forum

# Conflicts: # pylabrobot/hamilton/liquid_handlers/nimbus/chatterbox.py # pylabrobot/hamilton/liquid_handlers/nimbus/driver.py # pylabrobot/hamilton/liquid_handlers/nimbus/nimbus.py # pylabrobot/hamilton/liquid_handlers/nimbus/pip_backend.py

Reverts the HLC pieces of 8c929c6: - Delete pylabrobot/hamilton/liquid_handlers/liquid_class_resolver.py and its tests - STAR PIP backend: restore the local _resolve_liquid_classes helper - Nimbus PIP backend: drop all HLC fields, imports, and resolution/volume-correction logic - Delete the HLC Nimbus pip_backend_tests.py (was added entirely by that commit) The underlying HamiltonLiquidClass and STAR calibration tables predated the PR and are kept. Nimbus is restored to its pre-HLC state (no liquid-class support) with the _on_setup signature updated to accept backend_params for the capability protocol. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

cmoscy · 2026-04-19T23:05:31Z

Just to clarify, did you want me to pull hlc-related behavior out before merging this? Or just work on moving away from hlc behaviors as this gets built out alongside prep from this branch?

rickwierenga · 2026-04-19T23:17:04Z

Did that already actually, just forgot to push ... will do when I get home

Do you want to keep working on this branch or should I merge it? I thought this just adds/refactors TCP and some nimbus stuff and then we rebase the prep branch onto v1b1 after merging this one. But either way works

cmoscy · 2026-04-19T23:19:05Z

Did that already actually, just forgot to push ... will do when I get home

Do you want to keep working on this branch or should I merge it? I thought this just adds/refactors TCP and some nimbus stuff and then we rebase the prep branch onto v1b1 after merging this one. But either way works

Ah, no prob. Merging is probably easiest. I already split off the prep into a separate branch from here anyway. But whatever is most convenient.

- Delete docs/user_guide/.../nimbus_v1_aspirate_dispense.ipynb and prune its toctree entry. - Delete NimbusResolvedInterfaces dataclass, `_nimbus_resolved` / `_resolved_interfaces` instance state, and the `nimbus_interfaces` property — never read externally. Setup consumes the resolution dict locally. - Move build_parameters logic into NimbusCommand with an is_dataclass() guard, dropping 10 identical `return self._build_structured_parameters()` overrides and the helper. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

cmoscy · 2026-04-27T05:51:32Z

When there's a good time. Is there anything else you'd want reviewed/changed before merging this in?

Prep v1 builds on top of this, so would be easiest to merge this -> then open prep pr.

But can always rebase prep_v1 if that's best too. Just let me know.

Asking since Prep is ready :)

rickwierenga

there is a lot to learn here so just a first pass

rickwierenga · 2026-04-27T22:19:00Z

+    error_codes: Optional[Dict[Tuple[int, int, int, int, int], str]] = None,
  ):
+    merged_error_codes = {**NIMBUS_ERROR_CODES, **(error_codes or {})}


I do not understand why error codes would be passed through init

Agreed, addressed in changes. Now baked in to each respective backend

rickwierenga · 2026-04-27T22:26:59Z

+          if raise_on_error:
+            summary = ", ".join(
+              (
+                f"entry[{idx}]: {entry_errors[idx]} ({context_by_idx[idx]})"
+                if context_by_idx.get(idx)
+                else f"entry[{idx}]: {entry_errors[idx]}"
+              )
+              for idx in sorted(entry_errors)
+            )
+            logger.error(
+              "Hamilton %s (action=%#x), instrument-wide error (%d entries): %s",
+              action.name,
+              action,
+              len(entries),
+              summary,
+            )
+            raise HoiError(
+              exceptions=entry_errors,
+              entries=list(entries),
+              raw_response=response_message.hoi.params,
+            )


I think we should always raise_on_error. If the caller wants to not raise they can try/except. This is more readable and simple

Will trace through this logic carefully, as the introspection interface originally needed this (as did error resolution) when I had the partial type / incomplete communication layer and needed fallbacks for things that couldn't be 100% resolved. It could probably be simplified now, but will get back to you on that

rickwierenga · 2026-04-27T22:31:13Z

+def inspect_hoi_params(params: bytes) -> List[dict]:
+  """Inspect raw HOI params bytes fragment-by-fragment for debugging.



params as the name for the buffer is a bit weird because it also returns parameters, just a different format. Maybe param_buffer?

rickwierenga · 2026-04-27T22:39:08Z

+  async def get_structs_raw(self, address: Address, interface_id: int) -> tuple[bytes, List[dict]]:
+    """Get raw GetStructs response bytes and a fragment-by-fragment breakdown.
+
+    Use this to see exactly what the device sends so response parsing can
+    match the wire format. Returns (params_bytes, inspect_hoi_params(params)).
+
+    Example:
+      raw, fragments = await intro.get_structs_raw(mph_addr, 1)
+      for i, f in enumerate(fragments):
+        print(f\"{i}: type_id={f['type_id']} len={f['length']} decoded={f['decoded']!r}\")
+    """
+    command = GetStructsCommand(address, interface_id)
+    result = await self.backend.send_command(command, ensure_connection=False, return_raw=True)
+    (params,) = result
+    return params, inspect_hoi_params(params)


this method is not called anywhere afaict. Also, why does it need to return both paras and the dicts? don't the dicts contain all information? if not, then maybe it should be parsed by the caller.

this goes down a rabbit hole with inspect_hoi_params having untyped dictionaries but maybe it's not even used?

rickwierenga · 2026-04-27T22:45:55Z

+@dataclass
+class FirmwareTree:
+  """Structured firmware tree produced by introspection traversal."""
+
+  roots: List[FirmwareTreeNode] = field(default_factory=list)
+
+  def format(self) -> str:


are there really multiple tree roots? that is super cursed

Mistake on my part, addressed in changes

So if that is the case, then wouldn't it be possible to delete FirmwareTree and just keep FirmwareTreeNode?

You're suggesting we just build the interface tree using FirmwareTreeNode as the building block?

If I understand correctly that totally agree, much simpler with current behavior

yes, if it's really a tree then all nodes should be equal or at least be able to use the same class

Got it, will simplify that

rickwierenga · 2026-04-27T22:48:25Z

+  ids: tuple[int, int, int, int]  # [In, Out, InOut, RetVal]
+
+
+_HOI_TYPE_ROWS: tuple[_HoiTypeRow, ...] = (
+  _HoiTypeRow("i8", "i8", (1, 17, 9, 25)),
+  _HoiTypeRow("i16", "i16", (3, 19, 11, 27)),
+  _HoiTypeRow("i32", "i32", (5, 21, 13, 29)),


what are these ints? I thought HamiltonDataType but for example 17 doens't exist

The introspection type id system is separate from the wire for interface 0 methods. And type ids for introspection actually encode data type AND directionality. IN/OUT/INOUT/ReturnValue.

I hate it but is part of the introspection-specific typing system

I see, that's odd.

Is there a structure to this? would be more readable than the list of ints 😂

From the raw introspection responses, it's all ints. But open to anything that you'd think is an improvement here.

is this loaded from the machine or defined somewhere? do we know what these numbers map to?

rickwierenga · 2026-04-27T22:49:39Z

+class HamiltonTCPIntrospectionBackend(Protocol):
+  """Structural type for objects passed to :class:`HamiltonIntrospection`.
+
+  **Production:** :class:`~pylabrobot.hamilton.tcp.client.HamiltonTCPClient` implements this
+  Protocol (transport, registry, session caches, ``send_command``).
+
+  **Tests:** provide a minimal object with the same methods/properties so introspection can be
+  exercised without a socket—see ``tcp_tests`` (e.g. fake ``Backend`` with registry roots and
+  patched ``HamiltonIntrospection`` methods). This is a typing contract only; there is no separate
+  runtime "backend" class besides the client.
+  """
+
+  @property
+  def registry(self) -> Any: ...
+
+  def get_root_object_addresses(self) -> list[Address]: ...
+
+  @property
+  def global_object_addresses(self) -> Sequence[Address]: ...
+
+  async def send_command(
+    self,
+    command: TCPCommand,
+    *,
+    ensure_connection: bool = True,
+    return_raw: bool = False,
+    raise_on_error: bool = True,
+    read_timeout: Optional[float] = None,
+  ) -> Any: ...
+
+  async def resolve_path(self, path: str) -> Address: ...


why does this exist? I can only see it changing in testing, but that might be better done using a mock. I don't think in "normal" operation it would change what the "backend" is? that could really simplify things

This works as a standalone interface to arbitrary TCP-based Hamilton instruments that may or may not have the instrument specific backend implemented yet. The MPE2 for example from last meeting. Or when I first connected to the prep.

Just a centralized place to hold tcp logic, gets wrapped into a dependency injection (.driver) once there is a concrete backend for a specific instrument.

So not just for testing, otherwise I'd agree a mock it 100% a better design.

Given this, do you still think a consolidation/mock is the better pattern?

Yes, I understood that part and it's obviously the right choice to share an object for all hamilton tcp stuff.

If the firmware interfaces are as similar as I understand, shouldn't it be possible to have a lower level "driver" own the io object and just expose these send_command command and get_root_object_addresses etc. live on there? And then the NimbusDriver just owns that object, and the MPEDriver will have an instance of it as well.

Ah, actually my mistake, this can be refactored out as you're describing. Been staring at it so long it sort of merged into HamiltonTCPClient in my mind. Doh

love that name! let's have HamiltonTCPClient, and then NimbusDriver.client and Other.client in the future. And this can be nicely mocked out for testing.

rickwierenga · 2026-04-27T22:51:11Z

+    "year": ParameterType(type_id=5),  # U16
+    "month": ParameterType(type_id=4),  # U8 (padded)
+    "day": ParameterType(type_id=4),
+    "hour": ParameterType(type_id=4),


this problem exists in a lot of places, just commenting here, but ints are really confusing for type ids because it's not readable. why not just use HamiltonDataType.UI16 for example?

100% valid, addressed here, but will do a more comprehensive search and apply

wondering if we can get rid of ParameterType entirely and just use HamiltonDataType. Is it correct that ParameterType just provides some more information about the type? If so we can just store/compute that on the type directly

Will take a look! Probably a good idea.

…tency

…imbus alignment with Prep

cmoscy and others added 10 commits April 16, 2026 15:44

Hamilton TCP Layer: v1 Migration

4cb96ee

refactor tcp backend: centralize introspection ownership and simplify…

52c2a79

… HOI mapping

Merge Nimbus v1b1 branch:

e1dcc60

- NimbusDriver on HamiltonTCPClient with NIMBUS_ERROR_CODES; chatterbox, door, pip backend - TCPCommand as the shared wire command base; client/introspection aligned - Nimbus unit tests for error-code merge and interface resolution

Nimbus: Normalize interface discovery

f47f301

Hamilton: shared liquid-class resolver; Nimbus PIP HLC defaults; TCP …

8c929c6

…Nimbus errors & status exceptions

refactor(hamilton/tcp): replace status_exception with HoiError module

12f6d33

Move HOI error parsing (parse_hamilton_error_*) from messages into hoi_error. Pass HOI entries and per-entry exceptions into ChannelizedError for callers.

Nimbus example notebook

59feb5b

Formatting and PEP

8c6ea09

Merge remote-tracking branch 'origin/v1b1' into nimbus-v1-tcp-aligned

7f5ec82

Simplify Nimbus setup params handling to isinstance-only check

3316ab0

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

rickwierenga reviewed Apr 18, 2026

View reviewed changes

Comment thread pylabrobot/hamilton/liquid_handlers/prep/prep_commands.py

rickwierenga and others added 2 commits April 18, 2026 14:22

rickwierenga force-pushed the v1b1 branch from feee0ba to 162dd2c Compare April 23, 2026 17:30

cmoscy closed this Apr 27, 2026

cmoscy reopened this Apr 27, 2026

rickwierenga reviewed Apr 27, 2026

View reviewed changes

cmoscy added 3 commits April 28, 2026 11:40

Update from Prep-v1 branch: Introspection API improvements and consis…

e794c25

…tency

TCP and Introspection cleanup and in progress Nimbus alignment

b34c7f3

Introspection: Collapse firmwaretree into firmwarenode. In-progress N…

72367a2

…imbus alignment with Prep

		def inspect_hoi_params(params: bytes) -> List[dict]:
		"""Inspect raw HOI params bytes fragment-by-fragment for debugging.

Conversation

cmoscy commented Apr 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

rickwierenga commented Apr 18, 2026

Uh oh!

cmoscy commented Apr 18, 2026

Uh oh!

rickwierenga commented Apr 18, 2026

Uh oh!

cmoscy commented Apr 18, 2026

Uh oh!

cmoscy commented Apr 19, 2026

Uh oh!

rickwierenga commented Apr 19, 2026

Uh oh!

cmoscy commented Apr 19, 2026

Uh oh!

cmoscy commented Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rickwierenga left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

cmoscy commented Apr 18, 2026 •

edited

Loading

cmoscy commented Apr 27, 2026 •

edited

Loading