update sygnal_can_interface for handling HPO state feedback according…

sygnal_can_interface/sygnal_can_interface_lib/include/sygnal_can_interface_lib/sygnal_hpo_interface.hpp

@@ -25,10 +25,10 @@
 namespace polymath::sygnal
 {
 
-// HPO hardware exposes 5 interfaces (0-4). The DBC defines 7 signals (0-6) but the
-// remaining two are unused / always zero on real boards; we ignore them, mirroring the
-// same DBC-vs-hardware mismatch already handled by the MCM heartbeat parser.
-constexpr uint8_t HPO_NUM_INTERFACES = 5;
+// HPO hardware exposes 4 interfaces (0-3). The heartbeat reports a 1-bit control-state per
+// interface (Interface{N}State) plus a 2-bit pin-pull value (Interface{N}Value:
+// HiZ/Pull_Down/Pull_Up); the pull values are not yet surfaced (TODO: vehicleFeedback).
+constexpr uint8_t HPO_NUM_INTERFACES = 4;
 
 /// @brief Parsed HPO ControlEnableResponse / ControlCommandResponse.
 ///        is_enable_response disambiguates which payload is meaningful:
@@ -56,15 +56,15 @@ struct HpoErrorStatus
 
 /// @brief Self-contained HPO board representation.
 ///
-/// Owns the per-device interface bits (7 per-interface + 1 overall) and produces /
+/// Owns the per-interface control-state bits (one per HPO interface, 0-3) and produces /
 /// consumes the CAN frames the HPO understands. Frames are routed to the right
 /// instance by `bus_address_`: every parse helper rejects frames that do not match
 /// this device's bus address. Command frames are populated with `bus_address_`
 /// automatically so callers cannot mismatch addresses.
 ///
-/// Unlike the MCM, the HPO does not hold a Sygnal state machine. The per-interface
-/// and overall-interface heartbeat signals are 1-bit booleans (true = HPO in control,
-/// false = released). The MCM's SystemState byte is not tracked here.
+/// Unlike the MCM, the HPO does not hold a Sygnal state machine. Each per-interface
+/// heartbeat signal is a 1-bit boolean (true = HPO in control, false = released). The
+/// MCM's SystemState byte is not tracked here.
 class SygnalHpoInterface
 {
 public:
@@ -114,11 +114,6 @@ class SygnalHpoInterface
     return hpo_interface_states_;
   }
 
-  bool get_overall_interface_state() const
-  {
-    return hpo_overall_interface_state_;
-  }
-
   std::chrono::system_clock::time_point get_last_heartbeat_timestamp() const
   {
     return last_heartbeat_timestamp_;
@@ -127,7 +122,6 @@ class SygnalHpoInterface
 private:
   uint8_t bus_address_;
   std::array<bool, HPO_NUM_INTERFACES> hpo_interface_states_;
-  bool hpo_overall_interface_state_;
   std::chrono::system_clock::time_point last_heartbeat_timestamp_;
 };
 

sygnal_can_interface/sygnal_can_interface_lib/include/sygnal_can_interface_lib/sygnal_interface_socketcan.hpp

@@ -203,10 +203,6 @@ class SygnalInterfaceSocketcan
   /// @return std::nullopt if no HPO with this bus_address has been registered.
   std::optional<std::array<bool, HPO_NUM_INTERFACES>> get_hpo_interface_states(uint8_t bus_address) const;
 
-  /// @brief Read the cached overall interface bit from the named HPO's latest heartbeat.
-  /// @return std::nullopt if no HPO with this bus_address has been registered.
-  std::optional<bool> get_hpo_overall_interface_state(uint8_t bus_address) const;
-
 private:
   std::shared_ptr<socketcan::SocketcanAdapter> socketcan_adapter_;
   std::vector<SygnalMcmInterface> mcms_;

sygnal_can_interface/sygnal_can_interface_lib/src/sygnal_hpo_interface.cpp

@@ -28,14 +28,12 @@ namespace polymath::sygnal
 
 SygnalHpoInterface::SygnalHpoInterface()
 : bus_address_(0)
-, hpo_overall_interface_state_(false)
 {
   hpo_interface_states_.fill(false);
 }
 
 SygnalHpoInterface::SygnalHpoInterface(uint8_t bus_address)
 : bus_address_(bus_address)
-, hpo_overall_interface_state_(false)
 {
   hpo_interface_states_.fill(false);
 }
@@ -71,9 +69,7 @@ bool SygnalHpoInterface::parseHeartbeatFrame(const socketcan::CanFrame & frame)
   hpo_interface_states_[1] = (0 != unpacked.interface1_state);
   hpo_interface_states_[2] = (0 != unpacked.interface2_state);
   hpo_interface_states_[3] = (0 != unpacked.interface3_state);
-  hpo_interface_states_[4] = (0 != unpacked.interface4_state);
-  // Signals interface5_state / interface6_state are present in the DBC but unused on real HPO hardware.
-  hpo_overall_interface_state_ = (0 != unpacked.overall_interface_state);
+  // Interface{0-3}Value (2-bit pin-pull: HiZ/Pull_Down/Pull_Up) are also in this frame, not yet surfaced.
 
   return true;
 }

sygnal_can_interface/sygnal_can_interface_lib/src/sygnal_interface_socketcan.cpp

@@ -377,15 +377,4 @@ std::optional<std::array<bool, HPO_NUM_INTERFACES>> SygnalInterfaceSocketcan::ge
   return it->get_interface_states();
 }
 
-std::optional<bool> SygnalInterfaceSocketcan::get_hpo_overall_interface_state(uint8_t bus_address) const
-{
-  auto it = std::find_if(hpos_.begin(), hpos_.end(), [bus_address](const SygnalHpoInterface & h) {
-    return h.get_bus_address() == bus_address;
-  });
-  if (it == hpos_.end()) {
-    return std::nullopt;
-  }
-  return it->get_overall_interface_state();
-}
-
 }  // namespace polymath::sygnal

sygnal_can_interface/sygnal_can_interface_lib/test/sygnal_hpo_interface_test.cpp

@@ -59,7 +59,7 @@ polymath::socketcan::CanFrame makeFrame(uint32_t can_id, const uint8_t * data, u
 }
 
 polymath::socketcan::CanFrame buildHpoHeartbeat(
-  uint8_t bus_address, const std::array<bool, HPO_NUM_INTERFACES> & interface_states, bool overall_state)
+  uint8_t bus_address, const std::array<bool, HPO_NUM_INTERFACES> & interface_states)
 {
   hpo_heartbeat_heartbeat_t msg;
   hpo_heartbeat_heartbeat_init(&msg);
@@ -70,10 +70,6 @@ polymath::socketcan::CanFrame buildHpoHeartbeat(
   msg.interface1_state = interface_states[1] ? 1 : 0;
   msg.interface2_state = interface_states[2] ? 1 : 0;
   msg.interface3_state = interface_states[3] ? 1 : 0;
-  msg.interface4_state = interface_states[4] ? 1 : 0;
-  msg.interface5_state = 0;
-  msg.interface6_state = 0;
-  msg.overall_interface_state = overall_state ? 1 : 0;
   msg.count16 = 0;
   msg.crc = 0;
 
@@ -148,7 +144,6 @@ TEST_CASE("SygnalHpoInterface default constructor zeroes interface bits", "[sygn
 {
   SygnalHpoInterface hpo;
   REQUIRE(hpo.get_bus_address() == 0);
-  REQUIRE_FALSE(hpo.get_overall_interface_state());
   for (bool state : hpo.get_interface_states()) {
     REQUIRE_FALSE(state);
   }
@@ -158,7 +153,6 @@ TEST_CASE("SygnalHpoInterface explicit constructor sets bus address", "[sygnal_h
 {
   SygnalHpoInterface hpo(TEST_BUS_ADDRESS);
   REQUIRE(hpo.get_bus_address() == TEST_BUS_ADDRESS);
-  REQUIRE_FALSE(hpo.get_overall_interface_state());
   for (bool state : hpo.get_interface_states()) {
     REQUIRE_FALSE(state);
   }
@@ -167,11 +161,10 @@ TEST_CASE("SygnalHpoInterface explicit constructor sets bus address", "[sygnal_h
 TEST_CASE("SygnalHpoInterface parses heartbeat with all interfaces under HPO control", "[sygnal_hpo_interface]")
 {
   SygnalHpoInterface hpo(TEST_BUS_ADDRESS);
-  std::array<bool, HPO_NUM_INTERFACES> interfaces{true, true, true, true, true};
-  auto frame = buildHpoHeartbeat(TEST_BUS_ADDRESS, interfaces, true);
+  std::array<bool, HPO_NUM_INTERFACES> interfaces{true, true, true, true};
+  auto frame = buildHpoHeartbeat(TEST_BUS_ADDRESS, interfaces);
 
   REQUIRE(hpo.parseHeartbeatFrame(frame));
-  REQUIRE(hpo.get_overall_interface_state());
   for (bool state : hpo.get_interface_states()) {
     REQUIRE(state);
   }
@@ -180,11 +173,10 @@ TEST_CASE("SygnalHpoInterface parses heartbeat with all interfaces under HPO con
 TEST_CASE("SygnalHpoInterface parses heartbeat with mixed interface bits", "[sygnal_hpo_interface]")
 {
   SygnalHpoInterface hpo(TEST_BUS_ADDRESS);
-  std::array<bool, HPO_NUM_INTERFACES> interfaces{true, false, true, false, true};
-  auto frame = buildHpoHeartbeat(TEST_BUS_ADDRESS, interfaces, false);
+  std::array<bool, HPO_NUM_INTERFACES> interfaces{true, false, true, false};
+  auto frame = buildHpoHeartbeat(TEST_BUS_ADDRESS, interfaces);
 
   REQUIRE(hpo.parseHeartbeatFrame(frame));
-  REQUIRE_FALSE(hpo.get_overall_interface_state());
   auto states = hpo.get_interface_states();
   for (size_t i = 0; i < HPO_NUM_INTERFACES; ++i) {
     REQUIRE(states[i] == interfaces[i]);
@@ -194,19 +186,18 @@ TEST_CASE("SygnalHpoInterface parses heartbeat with mixed interface bits", "[syg
 TEST_CASE("SygnalHpoInterface rejects heartbeat with wrong frame ID", "[sygnal_hpo_interface]")
 {
   SygnalHpoInterface hpo(TEST_BUS_ADDRESS);
-  std::array<bool, HPO_NUM_INTERFACES> interfaces{true, true, true, true, true};
-  auto frame = buildHpoHeartbeat(TEST_BUS_ADDRESS, interfaces, true);
+  std::array<bool, HPO_NUM_INTERFACES> interfaces{true, true, true, true};
+  auto frame = buildHpoHeartbeat(TEST_BUS_ADDRESS, interfaces);
   frame.set_can_id(0x999);
 
   REQUIRE_FALSE(hpo.parseHeartbeatFrame(frame));
-  REQUIRE_FALSE(hpo.get_overall_interface_state());
 }
 
 TEST_CASE("SygnalHpoInterface rejects heartbeat with bad CRC", "[sygnal_hpo_interface]")
 {
   SygnalHpoInterface hpo(TEST_BUS_ADDRESS);
-  std::array<bool, HPO_NUM_INTERFACES> interfaces{true, true, true, true, true};
-  auto frame = buildHpoHeartbeat(TEST_BUS_ADDRESS, interfaces, true);
+  std::array<bool, HPO_NUM_INTERFACES> interfaces{true, true, true, true};
+  auto frame = buildHpoHeartbeat(TEST_BUS_ADDRESS, interfaces);
 
   // Corrupt the CRC byte.
   auto bytes = frame.get_data();
@@ -219,11 +210,10 @@ TEST_CASE("SygnalHpoInterface rejects heartbeat with bad CRC", "[sygnal_hpo_inte
 TEST_CASE("SygnalHpoInterface rejects heartbeat addressed to a different bus", "[sygnal_hpo_interface]")
 {
   SygnalHpoInterface hpo(TEST_BUS_ADDRESS);
-  std::array<bool, HPO_NUM_INTERFACES> interfaces{true, true, true, true, true};
-  auto frame = buildHpoHeartbeat(OTHER_BUS_ADDRESS, interfaces, true);
+  std::array<bool, HPO_NUM_INTERFACES> interfaces{true, true, true, true};
+  auto frame = buildHpoHeartbeat(OTHER_BUS_ADDRESS, interfaces);
 
   REQUIRE_FALSE(hpo.parseHeartbeatFrame(frame));
-  REQUIRE_FALSE(hpo.get_overall_interface_state());
 }
 
 TEST_CASE("SygnalHpoInterface creates a ControlEnable frame addressed to itself", "[sygnal_hpo_interface]")

sygnal_can_interface/sygnal_can_interface_lib/test/sygnal_interface_socketcan_routing_test.cpp

@@ -62,7 +62,7 @@ polymath::socketcan::CanFrame makeFrame(uint32_t can_id, const uint8_t * buffer,
   return frame;
 }
 
-polymath::socketcan::CanFrame buildHpoHeartbeat(uint8_t bus_address, bool overall_state)
+polymath::socketcan::CanFrame buildHpoHeartbeat(uint8_t bus_address)
 {
   hpo_heartbeat_heartbeat_t msg;
   hpo_heartbeat_heartbeat_init(&msg);
@@ -73,8 +73,6 @@ polymath::socketcan::CanFrame buildHpoHeartbeat(uint8_t bus_address, bool overal
   msg.interface1_state = 0;
   msg.interface2_state = 1;
   msg.interface3_state = 0;
-  msg.interface4_state = 1;
-  msg.overall_interface_state = overall_state ? 1 : 0;
   msg.count16 = 0;
   msg.crc = 0;
 
@@ -164,20 +162,15 @@ TEST_CASE("parse() routes HPO heartbeat to HPO state and leaves MCM untouched",
 
   polymath::sygnal::SygnalInterfaceSocketcan sygnal(adapter, mcms, hpos);
 
-  auto frame = buildHpoHeartbeat(HPO_BUS, /*overall_state=*/true);
+  auto frame = buildHpoHeartbeat(HPO_BUS);
   REQUIRE(sygnal.parse(frame));
 
-  auto hpo_state = sygnal.get_hpo_overall_interface_state(HPO_BUS);
-  REQUIRE(hpo_state.has_value());
-  REQUIRE(hpo_state.value());
-
   auto hpo_interfaces = sygnal.get_hpo_interface_states(HPO_BUS);
   REQUIRE(hpo_interfaces.has_value());
   REQUIRE((*hpo_interfaces)[0]);
   REQUIRE_FALSE((*hpo_interfaces)[1]);
   REQUIRE((*hpo_interfaces)[2]);
   REQUIRE_FALSE((*hpo_interfaces)[3]);
-  REQUIRE((*hpo_interfaces)[4]);
 
   // MCM state untouched: still default FAIL_HARD.
   auto mcm_state = sygnal.get_sygnal_mcm_state(MCM_BUS, 0);
@@ -200,10 +193,12 @@ TEST_CASE("parse() routes MCM heartbeat to MCM state and leaves HPO untouched",
   REQUIRE(mcm_state.has_value());
   REQUIRE(mcm_state.value() == polymath::sygnal::SygnalSystemState::MCM_CONTROL);
 
-  // HPO overall interface state stays at the default false.
-  auto hpo_state = sygnal.get_hpo_overall_interface_state(HPO_BUS);
-  REQUIRE(hpo_state.has_value());
-  REQUIRE_FALSE(hpo_state.value());
+  // HPO interface states stay at their default false.
+  auto hpo_interfaces = sygnal.get_hpo_interface_states(HPO_BUS);
+  REQUIRE(hpo_interfaces.has_value());
+  for (bool state : *hpo_interfaces) {
+    REQUIRE_FALSE(state);
+  }
 }
 
 TEST_CASE(