Skip to content

fix(vehicle): align navigation_gps_request order default across transports#83

Merged
Bre77 merged 2 commits into
mainfrom
fm/tfa-ble-nav-order-align
Jul 20, 2026
Merged

fix(vehicle): align navigation_gps_request order default across transports#83
Bre77 merged 2 commits into
mainfrom
fm/tfa-ble-nav-order-align

Conversation

@Bre77

@Bre77 Bre77 commented Jul 20, 2026

Copy link
Copy Markdown
Member

Intent

Align navigation_gps_request's order parameter across cloud (REST) and BLE transports per captain's ruling (2026-07-20): default an omitted order to 0 on both paths, rather than cloud omitting the field entirely while BLE required an int. Made order: int = 0 in both VehicleFleet.navigation_gps_request (fleet.py) and Commands.navigation_gps_request (commands.py). Verified 0 = REMOTE_NAV_TRIP_ORDER_UNKNOWN is a defined, valid proto enum value (proto/car_server.proto) safe on both transports' signed-command wire path - not a placeholder hack. Added cross-transport parity tests (tests/test_cross_transport_parity.py) asserting omitted order -> 0 on both cloud JSON payload and BLE protobuf message, and explicit order passes through unchanged. Updated AGENTS.md's cross-transport-parity note marking this divergence as resolved (previously listed as an open divergence pending decision). Full test suite, ruff, and pyright all pass.

What Changed

  • VehicleFleet.navigation_gps_request (fleet.py) and Commands.navigation_gps_request (commands.py) now both default order: int = 0 (REMOTE_NAV_TRIP_ORDER_UNKNOWN) when the caller omits it, replacing the prior mismatch where cloud omitted the field entirely (order: int | None = None) while BLE required a caller-supplied int.
  • Updated both methods' docstrings to note the new default, and updated AGENTS.md's cross-transport-parity note to mark this divergence resolved (was previously listed as open/pending decision).
  • Added NavigationGpsRequestParityTests to tests/test_cross_transport_parity.py asserting an omitted order resolves to 0 on both the cloud JSON payload and the BLE protobuf message, and that an explicit order still passes through unchanged.
  • Updated docs/bluetooth_vehicles.md and docs/fleet_api_signed_commands.md to reflect the new default; uv.lock updated incidentally.

Risk Assessment

✅ Low: Small, well-bounded signature change (order: int = 0 on both transports) verified against the proto enum, with cross-transport parity tests covering both the default and explicit-order paths, and no other call sites depend on the old None/required signature.

Testing

Ran the new and full test suites (364 passed) and manually exercised VehicleFleet.navigation_gps_request in a live Python REPL to confirm the actual outgoing REST JSON payload now includes order: 0 when omitted (previously the field would be absent), matching the BLE side's protobuf default verified by the new parity tests; also confirmed proto enum value 0 (REMOTE_NAV_TRIP_ORDER_UNKNOWN) is a legitimately defined value, not a placeholder. No issues found.

Evidence: Cloud REST payload before/after order default (manual REPL check)
cloud call, order omitted -> _request kwargs: {'lat': 37.323, 'lon': -122.0322, 'order': 0}
cloud call, order=2 -> _request kwargs: {'lat': 37.323, 'lon': -122.0322, 'order': 2}
Evidence: New cross-transport parity tests for navigation_gps_request
tests/test_cross_transport_parity.py::NavigationGpsRequestParityTests::test_explicit_order_passes_through_unchanged_on_both_transports PASSED
tests/test_cross_transport_parity.py::NavigationGpsRequestParityTests::test_omitted_order_defaults_to_zero_on_both_transports PASSED

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

✅ **Review** - passed

✅ No issues found.

✅ **Test** - passed

✅ No issues found.

  • uv run pytest tests/test_cross_transport_parity.py -v (6 passed, including the 2 new NavigationGpsRequestParityTests)
  • uv run pytest tests (364 passed, full suite)
  • Manual REPL check: called VehicleFleet.navigation_gps_request with order omitted and confirmed the outgoing _request json kwargs now contain order: 0 instead of omitting the key; called again with order=2 and confirmed pass-through
  • grep proto/car_server.proto to confirm REMOTE_NAV_TRIP_ORDER_UNKNOWN = 0 is a defined enum value on both NavigationGpsRequest and its sibling message
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

firstmate crewmate added 2 commits July 20, 2026 09:55
Cloud's order was optional (omitted -> null on the wire) while BLE's
was a required int. Default both to 0 (REMOTE_NAV_TRIP_ORDER_UNKNOWN,
a defined proto enum value) when the caller omits it, so cloud always
sends an explicit order instead of null.
@Bre77
Bre77 merged commit a287b54 into main Jul 20, 2026
5 checks passed
@Bre77 Bre77 mentioned this pull request Jul 20, 2026
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