fix(sensors): Share sync line control bitmasks (#799)

* add another singletop so sensor hardware instances can share their bitmask control

* update tests

* format

* remove prints

* use inverse operator

* fix pipette-simulators

gripper/firmware/main_proto.cpp

@@ -87,8 +87,11 @@ auto sensor_pins = sensors::hardware::SensorHardwareConfiguration{
                  .active_setting = GPIO_PIN_RESET}};
 
 auto version_wrapper = sensors::hardware::SensorHardwareVersionSingleton();
-auto sensor_hardware =
-    sensors::hardware::SensorHardware(sensor_pins, version_wrapper);
+static auto sync_control =
+    sensors::hardware::SensorHardwareSyncControlSingleton();
+
+auto sensor_hardware = sensors::hardware::SensorHardware(
+    sensor_pins, version_wrapper, sync_control);
 
 auto main() -> int {
     HardwareInit();

gripper/firmware/main_rev1.cpp

@@ -98,8 +98,11 @@ auto sensor_pins = sensors::hardware::SensorHardwareConfiguration{
                  .active_setting = GPIO_PIN_RESET}};
 
 auto version_wrapper = sensors::hardware::SensorHardwareVersionSingleton();
-auto sensor_hardware =
-    sensors::hardware::SensorHardware(sensor_pins, version_wrapper);
+static auto sync_control =
+    sensors::hardware::SensorHardwareSyncControlSingleton();
+
+auto sensor_hardware = sensors::hardware::SensorHardware(
+    sensor_pins, version_wrapper, sync_control);
 
 auto main() -> int {
     HardwareInit();

gripper/simulator/main.cpp

@@ -36,7 +36,9 @@ static auto sensor_map =
 static auto i2c2 = i2c::hardware::SimI2C{sensor_map};
 
 static sensors::hardware::SensorHardwareVersionSingleton version_wrapper{};
-static sim_mocks::MockSensorHardware fake_sensor_hw{version_wrapper};
+static sensors::hardware::SensorHardwareSyncControlSingleton sync_control{};
+static sim_mocks::MockSensorHardware fake_sensor_hw{version_wrapper,
+                                                    sync_control};
 
 static std::shared_ptr<state_manager::StateManagerConnection<
     freertos_synchronization::FreeRTOSCriticalSection>>

include/pipettes/firmware/utility_configurations.hpp

@@ -23,7 +23,8 @@ auto led_gpio(PipetteType pipette_type) -> gpio::PinConfig;
 
 auto get_sensor_hardware_container(
     SensorHardwareGPIO pins,
-    sensors::hardware::SensorHardwareVersionSingleton& version_wrapper)
+    sensors::hardware::SensorHardwareVersionSingleton& version_wrapper,
+    sensors::hardware::SensorHardwareSyncControlSingleton& sync_control)
     -> SensorHardwareContainer;
 
 template <PipetteType P>

include/sensors/core/sensor_hardware_interface.hpp

@@ -43,6 +43,67 @@ static auto get_mask_from_id(can::ids::SensorId sensor) -> uint8_t {
     return static_cast<uint8_t>(mask_enum);
 }
 
+class SensorHardwareSyncControlSingleton {
+  public:
+    SensorHardwareSyncControlSingleton() = default;
+    virtual ~SensorHardwareSyncControlSingleton() = default;
+    SensorHardwareSyncControlSingleton(
+        const SensorHardwareSyncControlSingleton&) = default;
+    auto operator=(const SensorHardwareSyncControlSingleton&)
+        -> SensorHardwareSyncControlSingleton& = default;
+    SensorHardwareSyncControlSingleton(SensorHardwareSyncControlSingleton&&) =
+        default;
+    auto operator=(SensorHardwareSyncControlSingleton&&)
+        -> SensorHardwareSyncControlSingleton& = default;
+
+    [[nodiscard]] auto mask_satisfied() const -> bool {
+        if (set_sync_required_mask !=
+            static_cast<uint8_t>(SensorIdBitMask::UNUSED)) {
+            // if anything is "required" only sync when they are all triggered
+            return (sync_state_mask & set_sync_required_mask) ==
+                   set_sync_required_mask;
+        }
+        return (sync_state_mask & set_sync_enabled_mask) != 0;
+    }
+
+    auto set_sync(can::ids::SensorId sensor) -> void {
+        // force the bit for this sensor to 1
+        sync_state_mask |= get_mask_from_id(sensor);
+    }
+
+    auto reset_sync(can::ids::SensorId sensor) -> void {
+        // force the bit for this sensor to 0
+        sync_state_mask &= ~get_mask_from_id(sensor);
+    }
+
+    auto set_sync_enabled(can::ids::SensorId sensor, bool enabled) -> void {
+        uint8_t applied_mask = get_mask_from_id(sensor);
+        if (!enabled) {
+            // force enabled bit to 0
+            set_sync_enabled_mask &= ~applied_mask;
+        } else {
+            // force enabled bit to 1
+            set_sync_enabled_mask |= applied_mask;
+        }
+    }
+
+    auto set_sync_required(can::ids::SensorId sensor, bool required) -> void {
+        uint8_t applied_mask = get_mask_from_id(sensor);
+        if (!required) {
+            // force required bit to 0
+            set_sync_required_mask &= ~applied_mask;
+        } else {
+            // force required bit to 1
+            set_sync_required_mask |= applied_mask;
+        }
+    }
+
+  private:
+    uint8_t set_sync_required_mask = 0x00;
+    uint8_t set_sync_enabled_mask = 0x00;
+    uint8_t sync_state_mask = 0x00;
+};
+
 class SensorHardwareVersionSingleton {
   public:
     SensorHardwareVersionSingleton() = default;
@@ -66,8 +127,9 @@ class SensorHardwareVersionSingleton {
 /** abstract sensor hardware device for a sync line */
 class SensorHardwareBase {
   public:
-    SensorHardwareBase(SensorHardwareVersionSingleton& version_wrapper)
-        : version_wrapper{version_wrapper} {}
+    SensorHardwareBase(SensorHardwareVersionSingleton& version_wrapper,
+                       SensorHardwareSyncControlSingleton& sync_control)
+        : version_wrapper{version_wrapper}, sync_control{sync_control} {}
     virtual ~SensorHardwareBase() = default;
     SensorHardwareBase(const SensorHardwareBase&) = default;
     auto operator=(const SensorHardwareBase&) -> SensorHardwareBase& = delete;
@@ -79,40 +141,27 @@ class SensorHardwareBase {
     virtual auto check_tip_presence() -> bool = 0;
 
     [[nodiscard]] auto mask_satisfied() const -> bool {
-        if (set_sync_required_mask !=
-            static_cast<uint8_t>(SensorIdBitMask::UNUSED)) {
-            // if anything is "required" only sync when they are all triggered
-            return (sync_state_mask & set_sync_required_mask) ==
-                   set_sync_required_mask;
-        }
-        return (sync_state_mask & set_sync_enabled_mask) != 0;
+        return sync_control.mask_satisfied();
     }
 
     auto set_sync(can::ids::SensorId sensor) -> void {
-        // force the bit for this sensor to 1
-        sync_state_mask |= get_mask_from_id(sensor);
+        sync_control.set_sync(sensor);
+        // update sync state now that requirements are different
         if (mask_satisfied()) {
             set_sync();
         }
     }
 
     auto reset_sync(can::ids::SensorId sensor) -> void {
-        // force the bit for this sensor to 0
-        sync_state_mask &= 0xFF ^ get_mask_from_id(sensor);
+        sync_control.reset_sync(sensor);
+        // update sync state now that requirements are different
         if (!mask_satisfied()) {
             reset_sync();
         }
     }
 
     auto set_sync_enabled(can::ids::SensorId sensor, bool enabled) -> void {
-        uint8_t applied_mask = get_mask_from_id(sensor);
-        if (!enabled) {
-            // force enabled bit to 0
-            set_sync_enabled_mask &= 0xFF ^ applied_mask;
-        } else {
-            // force enabled bit to 1
-            set_sync_enabled_mask |= applied_mask;
-        }
+        sync_control.set_sync_enabled(sensor, enabled);
         // update sync state now that requirements are different
         if (mask_satisfied()) {
             set_sync();
@@ -122,14 +171,7 @@ class SensorHardwareBase {
     }
 
     auto set_sync_required(can::ids::SensorId sensor, bool required) -> void {
-        uint8_t applied_mask = get_mask_from_id(sensor);
-        if (!required) {
-            // force required bit to 0
-            set_sync_required_mask &= 0xFF ^ applied_mask;
-        } else {
-            // force required bit to 1
-            set_sync_required_mask |= applied_mask;
-        }
+        sync_control.set_sync_required(sensor, required);
         // update sync state now that requirements are different
         if (mask_satisfied()) {
             set_sync();
@@ -142,10 +184,8 @@ class SensorHardwareBase {
     }
 
   private:
-    uint8_t set_sync_required_mask = 0x00;
-    uint8_t set_sync_enabled_mask = 0x00;
-    uint8_t sync_state_mask = 0x00;
     SensorHardwareVersionSingleton& version_wrapper;
+    SensorHardwareSyncControlSingleton& sync_control;
 };
 
 struct SensorHardwareContainer {

include/sensors/firmware/sensor_hardware.hpp

@@ -10,8 +10,10 @@ namespace hardware {
 class SensorHardware : public SensorHardwareBase {
   public:
     SensorHardware(sensors::hardware::SensorHardwareConfiguration hardware,
-                   SensorHardwareVersionSingleton& version_wrapper)
-        : SensorHardwareBase(version_wrapper), hardware(hardware) {}
+                   SensorHardwareVersionSingleton& version_wrapper,
+                   SensorHardwareSyncControlSingleton& sync_control)
+        : SensorHardwareBase(version_wrapper, sync_control),
+          hardware(hardware) {}
     auto set_sync() -> void override { gpio::set(hardware.sync_out); }
     auto reset_sync() -> void override { gpio::reset(hardware.sync_out); }
     auto check_tip_presence() -> bool override {

include/sensors/simulation/mock_hardware.hpp

@@ -12,8 +12,10 @@ namespace sim_mocks {
 class MockSensorHardware : public sensors::hardware::SensorHardwareBase {
   public:
     MockSensorHardware(
-        sensors::hardware::SensorHardwareVersionSingleton& version_wrapper)
-        : sensors::hardware::SensorHardwareBase{version_wrapper} {}
+        sensors::hardware::SensorHardwareVersionSingleton& version_wrapper,
+        sensors::hardware::SensorHardwareSyncControlSingleton& sync_control)
+        : sensors::hardware::SensorHardwareBase{version_wrapper, sync_control} {
+    }
     auto set_sync() -> void override {
         if (_state_manager) {
             _state_manager->send_sync_msg(SyncPinState::HIGH);

include/sensors/tests/mock_hardware.hpp

@@ -5,8 +5,10 @@ namespace test_mocks {
 class MockSensorHardware : public sensors::hardware::SensorHardwareBase {
   public:
     MockSensorHardware(
-        sensors::hardware::SensorHardwareVersionSingleton& version_wrapper)
-        : sensors::hardware::SensorHardwareBase{version_wrapper} {}
+        sensors::hardware::SensorHardwareVersionSingleton& version_wrapper,
+        sensors::hardware::SensorHardwareSyncControlSingleton& sync_control)
+        : sensors::hardware::SensorHardwareBase{version_wrapper, sync_control} {
+    }
     auto set_sync() -> void override {
         sync_state = true;
         sync_set_calls++;
@@ -17,7 +19,9 @@ class MockSensorHardware : public sensors::hardware::SensorHardwareBase {
     }
     auto check_tip_presence() -> bool override { return false; }
 
-    auto get_sync_state_mock() const -> bool { return sync_state; }
+    auto get_sync_state_mock() const -> bool {
+        return sensors::hardware::SensorHardwareBase::mask_satisfied();
+    }
     auto get_sync_set_calls() const -> uint32_t { return sync_set_calls; }
     auto get_sync_reset_calls() const -> uint32_t { return sync_reset_calls; }
 

pipettes/firmware/main.cpp

@@ -130,11 +130,15 @@ void encoder_callback(int32_t direction) {
 
 static auto version_wrapper =
     sensors::hardware::SensorHardwareVersionSingleton();
+
+static auto sync_control =
+    sensors::hardware::SensorHardwareSyncControlSingleton();
+
 static auto pins_for_sensor =
     utility_configs::sensor_configurations<PIPETTE_TYPE>();
 static auto sensor_hardware_container =
-    utility_configs::get_sensor_hardware_container(pins_for_sensor,
-                                                   version_wrapper);
+    utility_configs::get_sensor_hardware_container(
+        pins_for_sensor, version_wrapper, sync_control);
 
 static auto tip_sense_gpio_primary = pins_for_sensor.primary.tip_sense.value();
 

pipettes/firmware/utility_configurations.cpp

@@ -28,18 +28,19 @@ auto utility_configs::led_gpio(PipetteType pipette_type) -> gpio::PinConfig {
 
 auto utility_configs::get_sensor_hardware_container(
     utility_configs::SensorHardwareGPIO pins,
-    sensors::hardware::SensorHardwareVersionSingleton& version_wrapper)
+    sensors::hardware::SensorHardwareVersionSingleton& version_wrapper,
+    sensors::hardware::SensorHardwareSyncControlSingleton& sync_control)
     -> utility_configs::SensorHardwareContainer {
     if (pins.secondary.has_value()) {
         return utility_configs::SensorHardwareContainer{
-            .primary = sensors::hardware::SensorHardware(pins.primary,
-                                                         version_wrapper),
+            .primary = sensors::hardware::SensorHardware(
+                pins.primary, version_wrapper, sync_control),
             .secondary = sensors::hardware::SensorHardware(
-                pins.secondary.value(), version_wrapper)};
+                pins.secondary.value(), version_wrapper, sync_control)};
     }
     return utility_configs::SensorHardwareContainer{
-        .primary =
-            sensors::hardware::SensorHardware(pins.primary, version_wrapper)};
+        .primary = sensors::hardware::SensorHardware(
+            pins.primary, version_wrapper, sync_control)};
 }
 
 template <>

pipettes/simulator/main.cpp

@@ -304,11 +304,14 @@ int main(int argc, char** argv) {
     auto sim_eeprom = std::make_shared<eeprom::simulator::EEProm>(
         eeprom_model, options, TEMPORARY_PIPETTE_SERIAL);
     auto version_wrapper = sensors::hardware::SensorHardwareVersionSingleton();
+    auto sync_control = sensors::hardware::SensorHardwareSyncControlSingleton();
     auto fake_sensor_hw_primary =
-        std::make_shared<sim_mocks::MockSensorHardware>(version_wrapper);
+        std::make_shared<sim_mocks::MockSensorHardware>(version_wrapper,
+                                                        sync_control);
     fake_sensor_hw_primary->provide_state_manager(state_manager_connection);
     auto fake_sensor_hw_secondary =
-        std::make_shared<sim_mocks::MockSensorHardware>(version_wrapper);
+        std::make_shared<sim_mocks::MockSensorHardware>(version_wrapper,
+                                                        sync_control);
     fake_sensor_hw_secondary->provide_state_manager(state_manager_connection);
     auto pressuresensor_i2c1 = std::make_shared<mmr920_simulator::MMR920>();
     auto pressuresensor_i2c3 = std::make_shared<mmr920_simulator::MMR920>();

sensors/tests/test_capacitive_sensor.cpp

@@ -34,7 +34,8 @@ static std::array<float, SENSOR_BUFFER_SIZE> sensor_buffer;
 
 SCENARIO("read capacitance sensor values without shared CINs") {
     auto version_wrapper = sensors::hardware::SensorHardwareVersionSingleton();
-    test_mocks::MockSensorHardware mock_hw(version_wrapper);
+    auto sync_control = sensors::hardware::SensorHardwareSyncControlSingleton();
+    test_mocks::MockSensorHardware mock_hw(version_wrapper, sync_control);
     test_mocks::MockMessageQueue<i2c::writer::TaskMessage> i2c_queue{};
     test_mocks::MockMessageQueue<i2c::poller::TaskMessage> poller_queue{};
 
@@ -364,7 +365,8 @@ SCENARIO("read capacitance sensor values without shared CINs") {
 
 SCENARIO("read capacitance sensor values supporting shared CINs") {
     auto version_wrapper = sensors::hardware::SensorHardwareVersionSingleton();
-    test_mocks::MockSensorHardware mock_hw{version_wrapper};
+    auto sync_control = sensors::hardware::SensorHardwareSyncControlSingleton();
+    test_mocks::MockSensorHardware mock_hw{version_wrapper, sync_control};
     test_mocks::MockMessageQueue<i2c::writer::TaskMessage> i2c_queue{};
     test_mocks::MockMessageQueue<i2c::poller::TaskMessage> poller_queue{};
 
@@ -570,7 +572,8 @@ SCENARIO("read capacitance sensor values supporting shared CINs") {
 
 SCENARIO("capacitance driver tests no shared CINs") {
     auto version_wrapper = sensors::hardware::SensorHardwareVersionSingleton();
-    test_mocks::MockSensorHardware mock_hw{version_wrapper};
+    auto sync_control = sensors::hardware::SensorHardwareSyncControlSingleton();
+    test_mocks::MockSensorHardware mock_hw{version_wrapper, sync_control};
     test_mocks::MockMessageQueue<i2c::writer::TaskMessage> i2c_queue{};
     test_mocks::MockMessageQueue<i2c::poller::TaskMessage> poller_queue{};
 
@@ -756,7 +759,8 @@ SCENARIO("capacitance driver tests no shared CINs") {
 
 SCENARIO("threshold configuration") {
     auto version_wrapper = sensors::hardware::SensorHardwareVersionSingleton();
-    test_mocks::MockSensorHardware mock_hw{version_wrapper};
+    auto sync_control = sensors::hardware::SensorHardwareSyncControlSingleton();
+    test_mocks::MockSensorHardware mock_hw{version_wrapper, sync_control};
     test_mocks::MockMessageQueue<i2c::writer::TaskMessage> i2c_queue{};
     test_mocks::MockMessageQueue<i2c::poller::TaskMessage> poller_queue{};
 

sensors/tests/test_pressure_driver.cpp

@@ -49,12 +49,13 @@ SCENARIO("Testing the pressure sensor driver") {
     test_mocks::MockI2CResponseQueue response_queue{};
 
     auto version_wrapper = sensors::hardware::SensorHardwareVersionSingleton();
+    auto sync_control = sensors::hardware::SensorHardwareSyncControlSingleton();
 
     i2c::writer::TaskMessage empty_msg{};
     i2c::poller::TaskMessage empty_poll_msg{};
     auto writer = i2c::writer::Writer<test_mocks::MockMessageQueue>{};
     auto poller = i2c::poller::Poller<test_mocks::MockMessageQueue>{};
-    test_mocks::MockSensorHardware hardware{version_wrapper};
+    test_mocks::MockSensorHardware hardware{version_wrapper, sync_control};
     auto queue_client =
         mock_client::QueueClient{.pressure_sensor_queue = &pressure_queue};
     queue_client.set_queue(&can_queue);

sensors/tests/test_pressure_sensor.cpp

@@ -46,7 +46,8 @@ SCENARIO("Receiving messages through the pressure sensor message handler") {
     test_mocks::MockMessageQueue<sensors::utils::TaskMessage> pressure_queue{};
     test_mocks::MockI2CResponseQueue response_queue{};
     auto version_wrapper = sensors::hardware::SensorHardwareVersionSingleton();
-    test_mocks::MockSensorHardware mock_hw{version_wrapper};
+    auto sync_control = sensors::hardware::SensorHardwareSyncControlSingleton();
+    test_mocks::MockSensorHardware mock_hw{version_wrapper, sync_control};
 
     i2c::writer::TaskMessage empty_msg{};
     i2c::poller::TaskMessage empty_poll_msg{};