Standardize plate resource attributes (pf400 + sciclops) — design review

[Dozgulbas]

Opening this as a draft to review the standard_plate_resource_attr work (commit c8f3571) before it goes further, since there wasn't a PR yet.

The goal here — giving pf400 and sciclops distinct, unambiguous plate parameters — is a real problem worth solving, and the namespacing idea (pf400_grip_height vs sciclops_grip_height) is the right instinct. My review comments are mostly about where that standardization should live, plus a few correctness fixes.

Design discussion in #34.

[Dozgulbas]

Thanks for picking this up, Casey. The goal is right — pf400 and sciclops do need distinct, unambiguous plate parameters — and the namespaced keys (pf400_grip_height vs sciclops_grip_height) are a good way to get there. Removing the on-the-fly lid-resource creation in remove_lid is also the right direction.

My main piece of feedback is about PF400Plate. A plate isn't specific to pf400 or sciclops — it's a shared resource that's created at the workcell level and lives in the resource DB, and Resource/Asset are already Pydantic models with an attributes dict. So a module-local PF400Plate(BaseModel) doesn't actually become a MADSci resource type: it isn't stored in the DB, has no resource_id, and can't take part in the pick/place custody moves. It ends up as a separate in-memory copy of a resource we already have, and the pf400 and sciclops copies have already started to diverge.

We don't need it to hit the original goal: we can keep the way main already reads the resource and just add the namespaced attributes. The suggested approach is written up in #34, and there are a few correctness bugs in the inline comments worth fixing either way.

One question before we go further: how and where did you test this? A few paths look like they'd fail up front — the Python 3.10 import, and pick/place when a location has no attached resource — so I want to make sure we're testing the same setup.

[Dozgulbas]

A plate is shared labware: the workcell creates it and hands the action a resource_id, and the plate moves between instruments. get_resource() already returns a validated Asset/Slot with an attributes dict, and when the client reads from the DB it only rebuilds the standard resource types — so a custom class doesn't survive the round trip. This model can't be a real resource; it's a copy that can drift from the actual one.

Suggestion: drop this file and read attributes off the resource directly, the way main does. If we want typed access, a small helper function is fine — just not a model shaped like a resource. Full approach in #34.

[Dozgulbas]

Self is Python 3.11+, but the project supports >=3.10, so this fails to import on 3.10. Use from typing_extensions import Self if we keep it.

[Dozgulbas]

Nothing currently writes pf400_grip_height — the key only appears on the read side — so this raises KeyError for every existing plate until the resource definitions are updated to match. Also, the field is named grab_height_offset but read from pf400_grip_height, and we use it as an offset while sciclops uses grip_height as an absolute value; worth aligning the name and meaning. Best done as one coordinated key rename across pf400, sciclops, and the workcell defs.

[Dozgulbas]

This crashes when a location has no attached resource: plate_resource is None, so reading .grab_height_offset errors. main avoided that with a plain local value defaulting to 0.0. Since locations without a resource are supported, let's keep that path working — or, if we want to require resources, check for it early and return a clear error. Same pattern on lines 525 (pick) and 630 (place).

[Dozgulbas]

remove_lid now takes the lid from the plate's own lid_slot child, but startup_handler still defines pf400_lid_slot (a robot-side temporary slot) and plate_lid templates that nothing creates anymore — so "lid_slot" refers to two different things and those templates are unused. Since the lid handling isn't finished yet, I'd suggest keeping this branch focused on plate attributes and moving lids to a separate follow-up.

[Dozgulbas]

@caseystone — opened this draft to go over the plate-attribute standardization with you. There's a summary at the top plus a few inline notes, and #34 has the higher-level design discussion. The goal you're after is right; the comments are mostly about where it should live. No rush — happy to talk it through whenever.

[caseystone]

I created the PF400Plate class solely as a Pydantic model to make it easy to validate that the attributes required to perform PF400 pick-and-place actions are present and correctly defined in the MADSci plate resource at the source. It is not intended to replace the MADSci plate resource itself. Since the MADSci plate resource contains attributes that aren't relevant to the PF400 (such as SciClops-specific attributes), it made sense to parse only the PF400-specific fields while ignoring the rest.

Supporting plate resources that are missing the required attributes, as well as pick-and-place operations from locations that don't have a plate resource, is going to be a pain to maintain. We should talk about the best way to handle those cases going forward. For now, I removed support for them in this version with the goal of moving toward a more standardized, structured way of interacting with MADSci plate resources.