STARBackend: consolidate iSWAP drive parameters into iSWAPInformation#1055
Merged
BioCam merged 10 commits intoMay 21, 2026
Conversation
Add 13 firmware/hardware-version-dependent device-fact fields (per-drive area-of-operation increment ranges, encoder resolutions, speed/accel ranges) with 4th-generation defaults, and reorganize the whole record by axis/drive (X, Y, Z, rotation drive, wrist drive, gripper). The grouping is two-tiered because Python <3.10 dataclasses require non-default fields before defaulted ones, so per-machine EEPROM fields come first. Additive only: the new fields are unused so far and setup construction is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Switch every drive-bound check (move_y speed range; move_z position/speed/ acceleration ranges; the rotation and wrist range validators; the symmetric checks in `_iswap_rotate_increments`; the range checks in `iswap_rotate_to_angles`) to read from the `iSWAPInformation` range fields instead of the class constants. Behaviour-equivalent: the field defaults equal the constant literals. The now-unreferenced bound constants stay in place for now. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ion` Give the six public increment<->mm converters and the four private angle/resolve helpers an optional `mm_per_increment` / `deg_per_increment` parameter whose default is the corresponding `iSWAPInformation` field, and use the parameter in their bodies. The helpers stay static and pure, so they remain callable without a live backend. Additive and behaviour-equivalent: existing no-argument calls keep working and resolve to the same value (the dataclass default equals the old class constant). Internal callers are routed through the field in the next step. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Route the internal callers through the instance: converter call sites in the Y/Z/gripper request, move, and predefined-position methods now pass `self.iswap_information.<resolution>`, and the inline degree conversions (orientation tolerances/errors and the rotate-to-angles speed/acceleration math) read the field directly. After this the encoder-resolution class constants are unreferenced. Behaviour-equivalent. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Move the now-unreferenced drive constants out of their scattered per-drive positions into a single labeled block at the top of the iSWAP region, ordered to match iSWAPInformation (Y, Z, rotation drive, wrist drive, gripper). Each is tagged for removal at v1; their canonical home is now iSWAPInformation. The physical/structural constants (drive diameter, safety radius, finger-plane Z offset) stay in place. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drop the tautological one-line field docstrings (the field names, group headers, and `# unit:` comments already carry the information), condense the two-tier grouping note and the device-facts section header, and add the gripper joint label / jaw-width hint for symmetry. Rename the device-fact field `rotation_y_speed_increment_range` -> `y_speed_increment_range`: it lives in the Y group and is the Y-carriage speed range, so the `rotation_` prefix was misleading. The field is new in this series, so the rename breaks nothing; the vestigial class constant keeps its firmware-faithful name. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Use the repo's `# TODO: remove in v1` convention on the vestigial drive- constant block so it surfaces in TODO scans, and align the cross-reference in the iSWAPInformation device-facts header. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR consolidates iSWAP per-drive “device fact” parameters (increment ranges, encoder resolutions, speed/acceleration ranges) from scattered STARBackend class constants into the iSWAPInformation dataclass, so iSWAP behavior checks and unit conversions draw from a single configuration record.
Changes:
- Expanded
iSWAPInformationwith defaulted, generation-dependent device parameters (ranges/resolutions) alongside existing per-machine EEPROM calibration fields. - Migrated iSWAP drive validators and mm/deg↔increment converters to read ranges/resolutions from
self.iswap_information(and to accept optional override parameters). - Retained the superseded
STARBackendconstants in a single “TODO: remove in v1” compatibility block.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Check `min <= value <= max` against the full increment range instead of `abs(value) > max`, so the rotation and wrist range checks respect the configured lower bound rather than assuming symmetry around zero. Behaviour- equivalent for the current 4th-generation ranges (which are symmetric) and consistent with the other range validators; robust to an asymmetric future generation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
rickwierenga
approved these changes
May 21, 2026
setup() crashed with "iSWAP information not loaded": _iswap_rotation_drive_request_y_max runs while building iSWAPInformation, but predefined_y_positions read the not-yet-set iswap_information for its resolution. Give predefined_y_positions a y_mm_per_increment param defaulting to the class constant (matching the converter signatures), breaking the cycle. Verified on real STAR (serial 430-2621). Also wire the previously-dead gripper_increment_range into open/close validation: iswap_open_gripper now bounds open_position to the physical jaw range (~70.8-133.7 mm); iswap_close_gripper keeps the firmware 76.0 lower bound and replaces the meaningless 999.9 upper with the physical maximum. Adds one regression test for the setup-time bootstrap path (which the chatterbox setup bypasses, so CI didn't catch the crash).
burnpanck
reviewed
May 26, 2026
| @@ -1322,48 +1322,99 @@ class Head96Information: | |||
|
|
|||
| @dataclass | |||
Contributor
There was a problem hiding this comment.
Just two cents here: For this particular class, it might make sense to declare it as @dataclass(kw_only=True, frozen=True). frozen, because it really shouldn't be accidentally changed, and kw_only because IMHO, almost no dataclass should be instantiated via positional arguments anyway.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #1043. Moves the iSWAP per-drive device parameters (area-of-operation increment ranges, encoder resolutions, and speed/acceleration ranges) out of scattered
STARBackendclass constants and intoiSWAPInformation, so the record now holds everything known about the installed iSWAP: per-machine calibration plus firmware/hardware-version-dependent device facts. Behaviour-equivalent.Why
#1043 consolidated the per-machine setup state (link lengths, calibrated stops, offsets) into
iSWAPInformation, but the version-dependent device facts - drive increment ranges, encoder resolutions, speed/accel ranges - were still ~16 loose class constants onSTARBackend. These are "information about the iSWAP" too: they don't vary per unit, but they are firmware/hardware-generation-dependent (there is one known 4th-generation set today). They belong in the same record.Consolidating them gives one source of truth: validators and unit converters read the value from
iswap_informationinstead of a class constant, so a future generation needs a differentiSWAPInformationrather than scattered edits.Changes
STAR_backend.pyiSWAPInformation: +13 defaulted device-fact fields (per-drive increment ranges, degree/mm-per-increment resolutions, Y/Z speed ranges, Z acceleration range), with 4th-generation defaults. Fields reorganized by axis/drive (X, Y, Z, rotation drive, wrist drive, gripper).move_y/move_z, the rotation and wrist range validators,_iswap_rotate_increments,iswap_rotate_to_angles) now readsself.iswap_information.<range>.iSWAPInformationfield; internal callers pass the instance value. The helpers stay static and pure (callable without a live backend), and existing no-argument calls resolve to the same value.main) in one# TODO: remove in v1block, ordered to matchiSWAPInformation. The structural constants (drive diameter, safety radius, finger-plane Z offset) stay in place._set_up_iswapconstruction is unchanged.STAR_chatterbox.py_DEFAULT_ISWAP_INFORMATIONis unchanged - it inherits the new defaulted fields automatically.Behaviour-equivalent by construction (defaults equal the prior constant literals exactly) and backward-compatible (all retained constants stay accessible; converter signatures gained only optional defaulted parameters).
ruff,mypy, and the full test suite pass.Part 20 of the "Tame the iSWAP" epic:
https://discuss.pylabrobot.org/t/intro-to-epic-tame-the-iswap/517
🤖 Generated with Claude Code