Skip to content

fix(interface): handle v188 MIT feedback and gripper mode#83

Closed
JafarAbdi wants to merge 2 commits into
mainfrom
fix-v188-piperh-mode-feedback
Closed

fix(interface): handle v188 MIT feedback and gripper mode#83
JafarAbdi wants to merge 2 commits into
mainfrom
fix-v188-piperh-mode-feedback

Conversation

@JafarAbdi

Copy link
Copy Markdown
Contributor

No description provided.

@greptile-apps

greptile-apps Bot commented Jul 8, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes two related issues in the Piper gripper interface: it adds a MoveMode._missing_ handler so that firmware v188's 0x06 feedback value is correctly interpreted as MIT mode rather than raising an error, and it adds runtime detection of angle vs. width gripper mode so the correct control code (0x05/0x06 for angle, 0x01/0x02 for width) is sent on every gripper operation. A default effort of 1.0 Nm is also now applied in command_gripper when a position is given but no effort is specified.

  • MoveMode._missing_ silently maps 0x06MoveMode.MIT, preventing crashes when v188 firmware echoes the non-standard value.
  • Two new helpers (_gripper_uses_angle_mode, _gripper_code) centralise width/angle mode dispatch; all three gripper commands (enable_gripper, disable_gripper, command_gripper) are updated to call them.
  • command_open and command_close in piper_control.py now explicitly pass DEFAULT_GRIPPER_EFFORT, matching the new default-effort behaviour added inside command_gripper.

Confidence Score: 5/5

Safe to merge; the changes are narrowly scoped to firmware compatibility and gripper mode dispatch with no regressions on the arm path.

The MIT feedback remap and angle/width mode dispatch are correct. The only concern is that _GRIPPER_EFFORT_DEFAULT in piper_interface.py and DEFAULT_GRIPPER_EFFORT in piper_control.py are independent constants with the same value — a future edit to one without the other would silently produce differing efforts across callers, but this is not a current defect.

The duplicate default-effort constants in piper_interface.py and piper_control.py are the only thing worth a second look.

Important Files Changed

Filename Overview
src/piper_control/piper_interface.py Adds MoveMode.missing to handle v188 firmware's 0x06 MIT feedback, two helpers (_gripper_uses_angle_mode / _gripper_code) for angle vs width mode dispatch, and a default-effort fallback in command_gripper. Logic is sound; a duplicate default-effort constant introduced here and the existing one in piper_control.py must be kept in sync.
src/piper_control/piper_control.py command_open and command_close now explicitly pass DEFAULT_GRIPPER_EFFORT; this is now redundant since command_gripper auto-applies the default when position is provided, but it is harmless and the change is correct.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Caller
    participant PiperInterface
    participant SDK as piper SDK

    Caller->>PiperInterface: command_gripper(position, effort)
    PiperInterface->>PiperInterface: "apply default effort if position set & effort is None"
    PiperInterface->>SDK: get_gripper_status() [_gripper_uses_angle_mode]
    SDK-->>PiperInterface: gripper_state.mode
    PiperInterface->>PiperInterface: "_gripper_code(ENABLE=0x01, angle=0x05)"
    PiperInterface->>SDK: GripperCtrl(position_int, effort_int, code, 0)

    Caller->>PiperInterface: enable_gripper()
    PiperInterface->>SDK: get_gripper_status() [read current angle]
    SDK-->>PiperInterface: grippers_angle
    PiperInterface->>SDK: get_gripper_status() [_gripper_uses_angle_mode]
    SDK-->>PiperInterface: gripper_state.mode
    PiperInterface->>PiperInterface: "_gripper_code(ENABLE=0x01, angle=0x05)"
    PiperInterface->>SDK: GripperCtrl(raw_angle, 0, code, 0)

    Caller->>PiperInterface: disable_gripper()
    PiperInterface->>SDK: get_gripper_status() [_gripper_uses_angle_mode]
    SDK-->>PiperInterface: gripper_state.mode
    PiperInterface->>PiperInterface: "_gripper_code(DISABLE=0x02, angle=0x06)"
    PiperInterface->>SDK: GripperCtrl(0, 0, code, 0)
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Caller
    participant PiperInterface
    participant SDK as piper SDK

    Caller->>PiperInterface: command_gripper(position, effort)
    PiperInterface->>PiperInterface: "apply default effort if position set & effort is None"
    PiperInterface->>SDK: get_gripper_status() [_gripper_uses_angle_mode]
    SDK-->>PiperInterface: gripper_state.mode
    PiperInterface->>PiperInterface: "_gripper_code(ENABLE=0x01, angle=0x05)"
    PiperInterface->>SDK: GripperCtrl(position_int, effort_int, code, 0)

    Caller->>PiperInterface: enable_gripper()
    PiperInterface->>SDK: get_gripper_status() [read current angle]
    SDK-->>PiperInterface: grippers_angle
    PiperInterface->>SDK: get_gripper_status() [_gripper_uses_angle_mode]
    SDK-->>PiperInterface: gripper_state.mode
    PiperInterface->>PiperInterface: "_gripper_code(ENABLE=0x01, angle=0x05)"
    PiperInterface->>SDK: GripperCtrl(raw_angle, 0, code, 0)

    Caller->>PiperInterface: disable_gripper()
    PiperInterface->>SDK: get_gripper_status() [_gripper_uses_angle_mode]
    SDK-->>PiperInterface: gripper_state.mode
    PiperInterface->>PiperInterface: "_gripper_code(DISABLE=0x02, angle=0x06)"
    PiperInterface->>SDK: GripperCtrl(0, 0, code, 0)
Loading

Reviews (2): Last reviewed commit: "Try random 💩" | Re-trigger Greptile

Comment thread src/piper_control/piper_interface.py
Comment on lines +220 to +224
@classmethod
def _missing_(cls, value: object) -> MoveMode | None:
if value == 0x06:
return cls.MIT
return None

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 No comment explaining why 0x06 maps to MIT

The _missing_ override silently remaps 0x06 to MoveMode.MIT without any in-code explanation. A reader unfamiliar with the v188 firmware quirk will be left wondering why 0x04 and 0x06 are treated as the same mode. A one-line comment referencing the firmware version or the upstream SDK issue would make this much easier to maintain.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Claude Code Fix in Codex

Comment thread src/piper_control/piper_interface.py
@JafarAbdi JafarAbdi closed this Jul 16, 2026
@JafarAbdi

Copy link
Copy Markdown
Contributor Author

See #85

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant