[VCU] event modes, energy metering#289
Conversation
matt-mekha
requested review from
AaronShet and
dhairs
and removed request for
a team
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces an event_mode parameter to the VCU parameters structure and configures it across various event-specific parameter files. It also updates default sensor calibration values, adjusts the default pedal curve exponent, disables regen by default, and switches the active parameters to endurance mode. The review feedback highlights a critical bug in endurance_params.h where overriding the .regen_linelock sub-struct implicitly resets its other fields to zero, and notes a logic error where regen should be enabled (false) instead of disabled (true) for endurance mode. Additionally, several unnecessary trailing backslashes outside of macro definitions should be removed across the parameter files to prevent compiler warnings or unexpected behavior.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| .regen_linelock = { \ | ||
| .disable = true, \ | ||
| }, \ | ||
| .event_mode = 4, \ |
There was a problem hiding this comment.
There are two major issues here:
- C99 Struct Overriding: Initializing
.regen_linelock = { ... }overrides the entireregen_linelocksub-struct. This means all other fields ofregen_linelock(such aspack_current_limit_a,pack_terminal_voltage_limit_v, etc.) will be implicitly initialized to0. To override only thedisablefield, you must use the nested designator.regen_linelock.disable. - Logic Error: The PR title is "regen off except for endurance". Since regen is disabled by default in
default_params.h(.disable = true), endurance mode should enable it by setting.regen_linelock.disable = false. Setting it totruehere keeps it disabled.
.regen_linelock.disable = false,
.event_mode = 4,| static const vcu_parameters_t acceleration_params = { VCU_DEFAULT_PARAMS }; | ||
| static const vcu_parameters_t acceleration_params = { | ||
| VCU_DEFAULT_PARAMS, | ||
| .event_mode = 1, \ |
There was a problem hiding this comment.
The backslash \ at the end of the line is unnecessary because this is a standard C struct initialization, not a preprocessor macro definition. Trailing backslashes outside of macros can lead to compiler warnings or unexpected line-splicing behavior if trailing whitespace is present.
.event_mode = 1,| static const vcu_parameters_t autocross_params = { VCU_DEFAULT_PARAMS }; | ||
| static const vcu_parameters_t autocross_params = { | ||
| VCU_DEFAULT_PARAMS, | ||
| .event_mode = 3, \ |
There was a problem hiding this comment.
The backslash \ at the end of the line is unnecessary because this is a standard C struct initialization, not a preprocessor macro definition. Trailing backslashes outside of macros can lead to compiler warnings or unexpected line-splicing behavior if trailing whitespace is present.
.event_mode = 3,| }, \ | ||
| .torque_map.pedal_curve_exponent = 1.0f, \ | ||
| .event_mode = 2, \ |
There was a problem hiding this comment.
The backslashes \ at the end of these lines are unnecessary because this is a standard C struct initialization, not a preprocessor macro definition. Trailing backslashes outside of macros can lead to compiler warnings or unexpected line-splicing behavior if trailing whitespace is present.
},
.torque_map.pedal_curve_exponent = 1.0f,
.event_mode = 2,
|
@claude review this. |
…ackside Stacks PR #289 (VCU mm/autox firmware: event-mode params per session + regen-off-except-endurance) + PR #291 (telemetry & dash for VCU event_mode toggle) into telemetry/trackside-live-fixes. Result is the READ path for Controls.event_mode (which params table the VCU is running) end-to-end: VCU 0x1C7 byte 6 -> cand decode -> publishd -> kafka-bridge flatten -> dashd CanData.event_mode -> driver-facing MODE pill Plus bridge now emits line_lock_enabled and bse3 (round-tripped through the schema-gen chain so they're in proto/sql/Prisma/ORM/generated_mapping/etc.). 0x1C9 VCU Energy Estimate packet (net_energy + regen_energy) also lands. Conflict resolutions: - BEVO/dashd/frontend/src/screens/ScreenOne.tsx: kept HEAD's lap-card + theme imports and ADDED the new eventModeLabel import from PR #291. - viewer_tool/src/app/page.tsx, trackside-live/TracksideApp.tsx, trackside-live/trackside.css: PR #291 doesn't actually modify these (verified with git diff origin/main...origin/mm/autox-telemetry -- those files). The conflicts were spurious 3-way-merge noise from the heavy divergence — took HEAD wholesale to preserve all the recent trackside polish work. Write path (Pi -> VCU mode switch) is still deferred per the bevo-mode-set-tx memory note — needs the four pieces and is blocked on VCU firmware.
matt-mekha
changed the title
mm/autox added event_mode (0x1C7) and net_energy/regen_energy (0x1C9) to the CSV but didn't run the full schema-gen chain, so the schema-drift CI was failing on stale artifacts. Pure regen of the non-firmware codegen outputs: - drivers/longhorn-lib/can.json + protobuf/can_packets.proto - BEVO can.json / can_packets.proto / sensor_data.desc / generated_mapping.rs - telemtry sensor.proto / orion.proto / Prisma / SQL / ORM models Firmware-side can_ids.h / can_ids.c are intentionally NOT regenerated here — Matt is updating those alongside the new packet handlers and will commit them separately.
cyberneel
requested review from
celinayang13 and
zohaib642
and removed request for
a team
Resolves a 2-hunk conflict in BEVO/dashd/main.rs in favour of the branch's reboot-durable layout paths (/var/lib/bevo-dash + create_dir_all) over main's stale /tmp tmpfs version from the #295 squash. main's write logic is a strict subset, so nothing is lost. Brings in #289 (event modes/energy), #295 (park screen), #297 (skidpad) so the PR diff is exactly the unmerged trackside work.
3 participants
No description provided.