Activate Electric Air Module (0x8E) for cell-level battery monitoring - #213

Thanks a lot for the contribution. The implementation is clean and I appreciate the detailed work on the EAM packet.

At the moment I am not fully sure what EAM adds over the existing HoTT mapping:

• Air-ESC (0x8C) already carries ESC-specific telemetry, including BEC voltage/current/temperature.
• General Air (0x8D) already carries battery/cell-related telemetry. For example, Smart ESC / Spektrum Smart Battery, which provides real per-cell voltages, is already mapped to General Air cells.
• For HW5/Kontronik, this PR only replicates average cell voltage across the EAM cell slots, so it is not true per-cell telemetry.

There is also a possible compatibility concern because some HoTT receivers may already use Electric Air (0x8E) for their own telemetry, and HoTT only allows one sensor of each type on the bus.

So I would like to better understand the specific benefit:

• Does your transmitter display EAM data in a way that Air-ESC + General Air cannot?
• Are some values missing or shown incorrectly with the current MSRC mapping?
• Is there a specific receiver/transmitter combination where EAM is required?

Also, since enable_eam defaults to false, msrc_link would need a corresponding GUI option before this is usable for normal users.

Thank you for the thoughtful review.

You're right that for HW5/Kontronik the cell data is replication, not true per-cell — that's a fair call. But I'd like to offer a different framing for the EAM addition that I think addresses your concerns:

The UniSens-E (a well-established HoTT sensor from SM Modellbau) lets the user pick one of GAM / EAM / ESC / Vario as the module role. That's how it coexists with other sensors on the bus — the HoTTv4 "one per type" rule means flexibility in which type you announce as is the actual feature.

msrc currently auto-publishes as ESC (when an ESC is configured) and GAM (when enable_lipo is set). For users who already have a UniSens-E or similar as GAM on their bus, msrc-as-GAM collides. EAM is the natural alternative slot, and that's the gap this PR fills.

So the use case is:

• User has a HW5 or Kontronik ESC providing total voltage + avg cell + cell count (what I have)
• User has another sensor as GAM (UniSens-E, JLog, INA3221 elsewhere)
• User wants msrc to announce additional electric-specific data without colliding with the GAM sensor
• enable_eam (defaults false) gives them that switch

That said — your enable_eam would be more honest as part of a broader pattern: making all msrc module roles (ESC / GAM / EAM / Vario) explicitly togglable like UniSens-E does, rather than the current implicit auto-enable. I'm happy to take that direction in a follow-up PR if it's welcome.

For this specific PR: would you accept it as-is (EAM as opt-in only), or would you prefer I close it and submit (a) a smaller PR that just routes HW5/Kontronik cells to general_air[], (b) the broader "make all module roles explicitly configurable" refactor, or (c) something else?

Thank you for the careful review — your concerns are valid, and I want to step back and propose a broader direction rather than defending the EAM-specific PR.

Looking at the established pattern in the HoTT ecosystem: UniSens-E (SM Modellbau) — probably the most widely used HoTT telemetry sensor in the community — lets the user pick one role for the sensor: GAM, EAM, ESC, or Vario. The HoTTv4 "one per type" rule isn't a limitation, it's the actual design contract: each smart sensor announces as one type, and several sensors can coexist on the bus by picking different types.

msrc currently auto-publishes on multiple types simultaneously (ESC + GAM + Vario), which is convenient when msrc is alone on the bus but causes collisions when combined with other sensors (UniSens-E, JLog).

Would you be open to a broader refactor where msrc adopts the same single-role pattern? Concretely:

• New config field hott_module_role = ESC | GAM | EAM | Vario
• msrc publishes only on the selected role; sensor data routes accordingly
• Backward compatibility via either (a) default = ESC + version migration for existing configs, or (b) keeping per-role enable flags with current auto-defaults so existing setups keep working

If yes, I'd close this PR and open a new one for the broader change. If you'd rather keep the current multi-role auto behavior, I'll close this PR and submit a smaller alternative that just routes HW5/Kontronik cell_voltage to general_air[HOTT_GENERAL_CELL_*] (using your existing INA3221/Smart-ESC/Spektrum cell path).

Either direction is fine — what would you prefer?

Thank you, this is a good and well-reasoned proposal.

I need to analyze it more carefully before deciding the right direction. I agree that the current multi-module HoTT behavior can be convenient, but it can also create conflicts when other HoTT sensors are present on the bus.

Your proposed broader approach may be better than adding EAM as a special case, but I want to review the implications for existing users, config compatibility, msrc_link, and the current ESC/GAM/Vario routing before making a decision.

So please give me a bit of time to think about it. I will review the HoTT mapping and then decide whether a broader configurable HoTT role/output model makes sense, or whether it is better to keep the current behavior and make only a smaller change.

No rush — take all the time you need. Architecture decisions are exactly the kind of thing worth thinking through carefully, especially when backwards-compatibility and the msrc_link binary protocol are involved.

If it would help your evaluation, I'm happy to:

• share the local prototype branch (rough but builds and runs end-to-end)
• post a screenshot of the GUI role-selector tab
• sketch a backwards-compat plan for existing flashed configs (e.g. legacy multi-role mode triggered by hott_module_role value 0xFF, or per-role enable flags as a hybrid)

Otherwise I'll just wait for your decision and act accordingly.

Pivoted the implementation to address your concerns more comprehensively. The PR now does what we discussed in the comments:

Multi-checkbox hott_modules_enabled bitmask instead of single enable_eam flag. User picks any combination of ESC / GAM / EAM / Vario via four checkboxes on the Receiver tab. Default is 0x0B (ESC + GAM + Vario, EAM off) — matches current msrc behaviour exactly, so users who don't touch the checkboxes notice nothing.

Backward-compatible at runtime. Existing flashed configs have 0x00 in that byte (it was previously spare7). The firmware treats 0x00 as "legacy auto" and applies the default 0x0B at startup, so no reflash or config wipe needed.

Implementation strategy — instead of gating each ESC/GAM/Vario auto-enable inline (which would have touched ~10 sites in set_config), I kept the existing auto-enable wiring intact and added a final masking pass at the end of set_config that clears is_enabled[HOTT_TYPE_*] for roles the user opted out of. Smaller diff, easier to reason about, easier to revert if a single role causes issues.

EAM frame kept from the previous revision — that part of the work is still useful regardless of how role selection happens.

This also gives users a way to coexist msrc with a UniSens-E or JLog on the same bus: uncheck the role the other sensor occupies. Aligns msrc with the multi-sensor coexistence story.

Let me know if you'd like this scope dialed back further, or if a different role-selection UI (radio buttons / single combo / per-role auto-detect) would fit better.

Heads-up: force-pushed an amend that adds protocol-conditional
visibility for the new gbHottRoles GroupBox. Without it the
checkboxes were shown for every receiver protocol, not just HOTT.
Same pattern as cbVarioAutoOffset / gbFuelPressure. Diff is
+8 lines in mainwindow.cpp.

Force-pushed again — this time a real bug found during hardware bring-up on a Pico.

The EAM struct additions in this PR (electric_air[HOTT_ELECTRIC_COUNT] in hott_sensors_t, plus the electric arrays in triggers_value_t / triggers_menu_t) enlarge hott_task's stack frame by roughly 110 words. That overflowed STACK_RX_HOTT (was 450 + STACK_EXTRA).

On real hardware the symptom was nasty and non-obvious: the overflow corrupted adjacent RAM, the USB task's data got clobbered, and the board failed USB enumeration entirely — Windows just reported "USB device not recognized", no COM port at all. It builds clean and looks fine in review; it only shows up when you flash it.

Fix in this push: STACK_RX_HOTT raised 450 -> 700 words (constants.h). With that the board boots and enumerates normally, and an EAM frame built from synthetic sweep values comes through cleanly.

Flagging it explicitly because this class of bug — a struct grows, the task stack doesn't — is invisible to code review and CI. Might be worth a general note for future HoTT frame changes.

Thanks for the update. I understand the goal better now, but I am still not fully convinced this should be merged as-is.

This is not fixing a current bug in MSRC; it adds a new configuration model for a relatively specific HoTT coexistence use case. That can be useful, but it also adds non-trivial complexity: new GUI state, a new persisted config field, legacy handling, an EAM packet path, new alarm/menu entries, extra stack usage, and more HoTT role combinations to test.

Before accepting this, I would prefer to test it on real hardware and confirm that the benefit is worth the added complexity. In particular, I want to verify whether using EAM instead of GAM actually preserves the useful telemetry in a practical setup, and whether the transmitter displays it in a way that is clearly better or necessary.

So for now I am not rejecting the idea, but I am not ready to merge it yet. I will need to test it and think a bit more about whether MSRC should support this level of HoTT role configurability, or whether it is better to keep the current simpler behaviour.