Skip to content

feat(vehicle): add navigation/messaging/media signed commands#92

Closed
Bre77 wants to merge 1 commit into
mainfrom
fm/tfa-cmd-nav-msg-media
Closed

feat(vehicle): add navigation/messaging/media signed commands#92
Bre77 wants to merge 1 commit into
mainfrom
fm/tfa-cmd-nav-msg-media

Conversation

@Bre77

@Bre77 Bre77 commented Jul 22, 2026

Copy link
Copy Markdown
Member

Intent

  • Part of a full-coverage pass wrapping every remaining CarServer.VehicleAction proto field - this slice covers navigation/messaging/media.
  • Add five new signed commands to Commands, following the existing wrapper pattern: upcoming_calendar_entries/take_drivenote (signed-command siblings of the existing REST-only VehicleFleet methods of the same name), plus video_request, navigation_route, get_messages.
  • upcoming_calendar_entries/take_drivenote reuse the exact REST method names/signatures - on VehicleSigned, MRO already prefers Commands over VehicleFleet for shared names, matching how door_lock, set_temps, etc. work today.
  • Purely additive - no existing signatures touched.
  • Tests in tests/test_ble_nav_messaging_media_commands.py cover the BLE-built signed message for each command; cross-transport parity tests added to tests/test_cross_transport_parity.py for the two REST-sibling commands.

Wraps five previously-unimplemented VehicleAction fields:
upcoming_calendar_entries and take_drivenote (signed-command siblings
of the existing REST-only VehicleFleet methods of the same name),
plus video_request, navigation_route, and get_messages. Both
Fleet-signed and BLE transports get these for free via the shared
Commands ABC.
@Bre77
Bre77 force-pushed the fm/tfa-cmd-nav-msg-media branch from e9f232d to fdbc41e Compare July 22, 2026 22:20

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fdbc41eb0c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +2564 to +2568
return await self._sendInfotainment(
Action(
vehicleAction=VehicleAction(
navigationRouteAction=NavigationRouteAction()
)

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 Badge Treat navigation_route as a read and return its payload

For callers using VehicleBluetooth(confirmation="optimistic"), this no-arg route request is currently treated as a mutating command, so the BLE override skips the reply wait and returns best-effort success instead of the vehicle's navigationRouteResponse; in the normal ack path the response is also dropped because Commands._command only returns ping, vehicleData, or actionStatus responses. This makes the new API unable to deliver the route/traffic data it requests; pass it as mutating=False and decode the navigationRouteResponse branch before returning.

Useful? React with 👍 / 👎.

Comment on lines +2575 to +2576
Action(vehicleAction=VehicleAction(getMessagesAction=GetMessagesAction())),
mutating=False,

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 Badge Return in-car messages instead of generic success

When the vehicle answers this read with a getMessagesResponse, Commands._command has no branch for that response oneof, so the new get_messages() method falls through to {"response": {"result": True, "reason": ""}} and discards every in-car message. A caller using this API will see success but never the messages it asked for; decode and return response.getMessagesResponse for this action.

Useful? React with 👍 / 👎.

@Bre77

Bre77 commented Jul 22, 2026

Copy link
Copy Markdown
Member Author

Folded into #91 to avoid serial merge conflicts across the same commands.py regions - closing this in favor of the consolidated PR.

@Bre77 Bre77 closed this Jul 22, 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