alfred: publish the drive train's integrated wheel odometry - #3620
alfred: publish the drive train's integrated wheel odometry#3620jeff-hykin wants to merge 3 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #3620 +/- ##
==========================================
- Coverage 77.26% 77.25% -0.01%
==========================================
Files 1262 1262
Lines 120302 120302
Branches 10600 10600
==========================================
- Hits 92953 92943 -10
- Misses 24284 24291 +7
- Partials 3065 3068 +3
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Greptile SummaryThis change adds Alfred wheel-odometry polling, publishes controller pose and derived twist data, and serializes controller requests. Two P1 reliability issues remain: shutdown can overlap an unfinished odometry request with the stop-and-close sequence, and one publication failure can stop all future wheel-odometry updates. Confidence Score: 3/5Not merge-safe until shutdown waits for the active controller request before closing the client and publication failures are contained within the polling loop. The score is 3 because two distinct P1 findings remain and neither is a security finding. Files Needing Attention: dimos/robot/diy/alfred/effector_high_level.py
 T-Rex LogsWhat T-Rex did
Reviews (3): Last reviewed commit: "wheel odometry: import portal lazily lik..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if self._odometry_task is not None and not self._odometry_task.done(): | ||
| self._odometry_task.cancel() | ||
| if self._stop_task is not None and not self._stop_task.done(): | ||
| self._stop_task.cancel() | ||
| try: | ||
| await self._send_velocity(0.0, 0.0, 0.0) | ||
| except Exception as e: | ||
| logger.error(f"Error stopping Alfred: {e}") | ||
| if self._client is not None: | ||
| try: | ||
| self._client.close() | ||
| except Exception: | ||
| pass | ||
| try: | ||
| client.close() |
There was a problem hiding this comment.
Cancelling the odometry task does not wait for an in-flight asyncio.to_thread(future.result) call to finish. Teardown then attempts the zero-velocity command and closes the shared Portal client while that odometry RPC still owns the multiplexed connection. The stop command can therefore fail, leaving a previously commanded non-zero velocity active after this module shuts down. Wait for the polling task's worker-backed request to finish, or otherwise serialize and complete the stop before closing the client.
Artifacts
Focused Alfred odometry teardown harness
- Authored mocked runtime harness that holds the Portal odometry future in flight, enters `main()` teardown, and records RPC ordering, stop outcome, and close timing.
Focused Alfred odometry teardown observed output
- Captured `uv run --no-sync` output showing stop failure from concurrent odometry RPC and client closure while the odometry worker remained blocked.
| self.wheel_odometry.publish( | ||
| Odometry( | ||
| ts=ts, | ||
| frame_id=self.config.wheel_odom_frame_id, | ||
| child_frame_id=self.config.base_frame_id, | ||
| pose=Pose( | ||
| position=Vector3(x, y, 0.0), | ||
| orientation=Quaternion.from_euler(Vector3(0.0, 0.0, yaw)), | ||
| ), | ||
| twist=twist, | ||
| ) | ||
| ) |
There was a problem hiding this comment.
wheel_odometry.publish() is outside the polling loop's exception handling. A publish failure propagates out of _poll_wheel_odometry, completes the background task, and prevents all future controller reads and wheel-odometry messages. Catch and log publication failures within the loop so the next polling interval can continue.
Artifacts
Focused mocked wheel-odometry harness source
- This authored uv runtime harness supplies successful odometry reads and fails the third publish once to exercise the exact polling path.
Wheel-odometry publish-failure runtime output
- This captured uv execution shows three successful reads, the injected publish exception, task completion, and no subsequent polling, confirming permanent loss of updates.
github-actions
Bot
added
the
ready-to-merge
The __init__ lock was immediately replaced in main(), the max(hz, 1e-3) guarded a config value that is never zero, and the poll docstring repeated what now lives as comments at the negation and twist sites.
github-actions
Bot
added
ready-to-merge
| try: | ||
| yield | ||
| finally: | ||
| if self._odometry_task is not None and not self._odometry_task.done(): |
There was a problem hiding this comment.
- Bug
- A focused mocked async harness loaded the current module and blocked getodometry....result in the production asyncio.tothread path. Shutting down main while that worker remained blocked reported stopoverlappedodometry=True and closeoverlappedodometry=True; the worker was still live when teardown returned. Releasing the worker before teardown in the control run produced no overlap, confirming that cancelling the polling task does not wait for its thread-backed controller request before stop and close use the same client.
- Cause
- T-Rex reproduced this while running the changed behavior, but it did not return a separate root-cause sentence.
- Fix
- Update the changed code so this failing path is handled, then rerun the same T-Rex check to confirm it passes.
Artifacts
Focused mocked async reproduction source
- This executable harness loads the current production module with minimal import stubs, blocks the production odometry future, and records whether teardown overlaps the stop RPC or close with that live worker.
Teardown while odometry worker is blocked
- Running the focused harness against current code shows both the stop RPC and close overlap the still-live odometry worker, confirming the teardown race.
Control run after odometry worker completes
- Running the same harness after releasing the worker before teardown shows no overlap, isolating the failure to teardown while the thread-backed RPC remains live.
| ) | ||
| previous = (ts, x, y, yaw) | ||
|
|
||
| self.wheel_odometry.publish( |
There was a problem hiding this comment.
- Bug
- A focused mocked async harness executed the current polling loop with a publisher that raised on its first call. The run observed controllerreads=1, publishcalls=1, taskdone=True, and the injected RuntimeError; no later poll or publication occurred. A repeated run produced the same result, confirming that an exception from wheelodometry.publish escapes the loop and terminates the background task.
- Cause
- T-Rex reproduced this while running the changed behavior, but it did not return a separate root-cause sentence.
- Fix
- Update the changed code so this failing path is handled, then rerun the same T-Rex check to confirm it passes.
Artifacts
Focused mocked async reproduction source
- The executable source loads the current poller and injects a controller plus publisher whose first publish fails, demonstrating the exact tested path.
First run showing publish exception ends odometry polling
- The first executed capture shows one read and one publish, then a completed task carrying the intentional RuntimeError, proving polling does not continue.
Repeated run showing publish exception ends odometry polling
- The repeated executed capture reproduces the same completed task after the first publish failure, confirming the behavior is deterministic.
portal ships in the misc extra, so a top-level import breaks blueprint validation tests in environments without it.
github-actions
Bot
added
ready-to-merge
| try: | ||
| yield | ||
| finally: | ||
| if self._odometry_task is not None and not self._odometry_task.done(): |
There was a problem hiding this comment.
- Bug
- Ran python3 trex-artifacts/alfred-poll-cancellation-harness.py control and python3 trex-artifacts/alfred-poll-cancellation-harness.py blocked against the current production source with mocked Portal futures and client. The control run released the odometry worker before teardown and observed no overlap. The blocked run observed workerblockedatshutdown=True, zerovelocityoverlap=True, and closeoverlap=True, confirming that cancelling the polling coroutine does not finish its thread-backed Portal request before the zero-velocity command and client close proceed.
- Cause
- T-Rex reproduced this while running the changed behavior, but it did not return a separate root-cause sentence.
- Fix
- Update the changed code so this failing path is handled, then rerun the same T-Rex check to confirm it passes.
Artifacts
Alfred cancellation overlap harness source
- Exact focused in-process harness that loads the current Alfred production source, blocks mocked `get_odometry().result()`, and records shutdown overlap; takeaway: it directly exercises the relevant current-code async path.
Released-worker control output
- Captured control execution where the mocked odometry worker completed before teardown and both zero velocity and close reported no overlap; takeaway: serialized shutdown is clean only once the worker has finished.
- Captured blocked-worker execution where teardown sent zero velocity and closed the mocked client while `get_odometry().result()` remained live; takeaway: the reported cancellation/close overlap is reproduced in current code.
| ) | ||
| previous = (ts, x, y, yaw) | ||
|
|
||
| self.wheel_odometry.publish( |
There was a problem hiding this comment.
- Bug
- Ran the focused async publish-exception harness with .venv/bin/python against the current polling loop, mocked controller futures, and normal and failing publishers. The control run recorded polls=2 publishes=2 taskdone=False; the injected first-publish-failure run recorded polls=1 publishes=1 taskdone=True and RuntimeError: injected publish failure. This confirms that an exception from wheelodometry.publish escapes the loop and permanently terminates polling.
- Cause
- T-Rex reproduced this while running the changed behavior, but it did not return a separate root-cause sentence.
- Fix
- Update the changed code so this failing path is handled, then rerun the same T-Rex check to confirm it passes.
Artifacts
Wheel-odometry publish exception harness
- Focused async in-process harness with mocked controller futures and normal/failing publishers that exercises the current poller behavior, takeaway: the reproduction is self-contained and executable with the repository virtual environment.
Wheel-odometry polling control output
- Observed control execution showing two successful polls and an unfinished polling task after normal publishes, takeaway: polling remains alive when publishing succeeds.
Wheel-odometry publish failure output
- Observed injected-first-publish-failure execution showing one poll, a completed task, and the escaping RuntimeError, takeaway: the exception permanently terminates polling.
1 participant
Wheel odom