Skip to content

Allow sygnal socket can interface to handle any number of MCMs#18

Open
Ryanbahl9 wants to merge 2 commits intomainfrom
ryan/refactor_to_allow_any_number_of_mcms
Open

Allow sygnal socket can interface to handle any number of MCMs#18
Ryanbahl9 wants to merge 2 commits intomainfrom
ryan/refactor_to_allow_any_number_of_mcms

Conversation

@Ryanbahl9
Copy link
Copy Markdown

Remove McmSystem wrapper; support arbitrary MCM configurations

sygnal_can_interface_lib

sygnal_mcm_interface.hpp

  • Added get_bus_address() and get_subsystem_id() public getters
  • The underlying fields already existed but were private with no accessor
  • Allows sygnal_interface_socketcan to look up interfaces by identity without needing an external index

sygnal_interface_socketcan.hpp

  • Removed struct McmSystem, which grouped two named members (mcm_0, mcm_1) under a shared bus address
  • Redundant since each SygnalMcmInterface already carries its own bus and subsystem IDs internally
  • Replaced with struct McmId { uint8_t bus_id; uint8_t subsystem_id; }, used only at construction time
  • std::array<McmSystem, 2> mcm_systems_ becomes std::vector<SygnalMcmInterface> mcms_
  • Constructor now takes const std::vector<McmId> &

sygnal_interface_socketcan.cpp

  • Constructor builds mcms_ by emplacing one SygnalMcmInterface(id.bus_id, id.subsystem_id) per entry
  • parse collapses the nested mcm_0/mcm_1 calls into a single range loop
  • get_interface_state and get_sygnal_mcm_state replace their hardcoded if (subsystem_id == 0) ... else if (subsystem_id == 1) ladders with std::find_if over the flat list using the new getters

sygnal_interface_socketcan_test.cpp

  • Updated the SygnalInterfaceSocketcan construction site to pass a McmId list equivalent to the old hardcoded defaults
  • Added a test case that configures a single bus with non-contiguous subsystem IDs {0, 2, 5}, verifies subsystem 5 is reachable after a heartbeat, and checks that unconfigured subsystem 1 correctly returns std::nullopt

sygnal_can_interface_ros2

The ROS2 node now drives the MCM topology from parameters. Two new parallel int_array params, mcm_bus_addresses (default [1, 1, 2, 2]) and mcm_subsystem_ids (default [0, 1, 0, 1]), describe the list of endpoints. The defaults reproduce the previous behavior so existing deployments need no config changes.

  • In on_configure, the two arrays are zipped into a std::vector<McmId> (with a length-mismatch guard) and passed to the SygnalInterfaceSocketcan constructor
  • mcm_heartbeat_entries_ is populated from the same list, replacing the four hardcoded assignments
  • Container type changes from std::array<McmHeartbeatEntry, 4> to std::vector<McmHeartbeatEntry>
  • All other node paths (timerCallback, createDiagnosticsMessage, lifecycle activate/deactivate/cleanup) already iterate the container generically and needed no changes

@eric-polymath
Copy link
Copy Markdown

eric-polymath commented May 6, 2026

For the case where there may not be a subsystem associated with an MCM (as is documented in LEV05 interface doc) it looks like the SygnalMcmInterface strictly expects two-args for the constructor (id.bus_id & id.subsystem_id). I guess the assumption is that if no subsystem ID is passed it will just get assigned subsystem ID = 0 which may be fine.

This comment is based on the assumption that the interface document is the source of truth and whether or not the MCM will always have an associated bus and subsystem ID.

Not blocking, but worth the discussion

@Ryanbahl9
Copy link
Copy Markdown
Author

For the case where there may not be a subsystem associated with an MCM (as is documented in LEV05 interface doc) it looks like the SygnalMcmInterface strictly expects two-args for the constructor (id.bus_id & id.subsystem_id). I guess the assumption is that if no subsystem ID is passed it will just get assigned subsystem ID = 0 which may be fine.

This comment is based on the assumption that the interface document is the source of truth and whether or not the MCM will always have an associated bus and subsystem ID.

Not blocking, but worth the discussion

In order to support LEV02, Sygnal added subsystem_id to all their CAN message definitions. See this PR. So it is safe to assume we will always need a subsystem ID because we now need to send one in the CAN message no matter what.

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.

2 participants