Skip to content

Conversation

@greatgitsby
Copy link
Contributor

@greatgitsby greatgitsby commented Nov 11, 2025

we will use these onroad events in mici UI, and the demo route still uses old deprecated onroadevents

modularized into separate methods since onroad events is a nasty migration

@github-actions github-actions bot added the tools label Nov 11, 2025
@greatgitsby greatgitsby changed the title feat(replay): migrate onroadevents feat(replay): migrate onroadevents, cleanup Nov 12, 2025
@greatgitsby greatgitsby marked this pull request as ready for review November 12, 2025 02:24
@greatgitsby greatgitsby marked this pull request as draft November 12, 2025 02:24
@greatgitsby greatgitsby marked this pull request as ready for review November 12, 2025 02:26
@greatgitsby
Copy link
Contributor Author

this is kinda messy. i see we only want to run migration on controlsState to selfdriveState if there are no selfdriveState messages in the segment.

i think it's probably best to have a separate method that always runs the onroadEventsDEPRECATED migration and leave the rest of the logic alone.

@greatgitsby greatgitsby marked this pull request as draft November 12, 2025 02:30
@greatgitsby greatgitsby marked this pull request as ready for review November 12, 2025 02:41
@deanlee
Copy link
Contributor

deanlee commented Nov 12, 2025

It might be cleaner and easier to maintain if we use a single mapping table instead of a massive switch statement.

Here’s a simplified approach using a small macro to reduce redundancy and improve readability:

using OldEventName = cereal::OnroadEventDEPRECATED::EventName;
using NewEventName = cereal::OnroadEvent::EventName;

#define MAP_EVENT_NAME(name) {OldEventName::name, NewEventName::name}

static const std::unordered_map<OldEventName, NewEventName> event_name_map = {
  MAP_EVENT_NAME(CAN_ERROR),
  MAP_EVENT_NAME(STEER_UNAVAILABLE),
  MAP_EVENT_NAME(WRONG_GEAR),
  MAP_EVENT_NAME(DOOR_OPEN),
  ...
};

@greatgitsby
Copy link
Contributor Author

cool! let me try this out

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants