Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
Aug 6, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions gripper/firmware/main_proto.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
7 changes: 5 additions & 2 deletions gripper/firmware/main_rev1.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
4 changes: 3 additions & 1 deletion gripper/simulator/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>>
Expand Down
3 changes: 2 additions & 1 deletion include/pipettes/firmware/utility_configurations.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
107 changes: 75 additions & 32 deletions include/sensors/core/sensor_hardware_interface.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,70 @@ 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)) {
printf("%x is required\n", set_sync_required_mask);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mmm don't like these even they compile out

printf("%x sync_state_mask\n", sync_state_mask);
// 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
printf("setting sync on %x sensor\n", get_mask_from_id(sensor));
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 &= 0xFF ^ get_mask_from_id(sensor);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
sync_state_mask &= 0xFF ^ get_mask_from_id(sensor);
sync_state_mask &= ~get_mask_from_id(sensor);

?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's the more common way I've seen, anyway

}

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;
}
}

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;
}
}

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;
Expand All @@ -66,8 +130,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;
Expand All @@ -79,40 +144,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();
Expand All @@ -122,14 +174,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();
Expand All @@ -142,10 +187,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 {
Expand Down
6 changes: 4 additions & 2 deletions include/sensors/firmware/sensor_hardware.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
6 changes: 4 additions & 2 deletions include/sensors/simulation/mock_hardware.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
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);
Expand Down
10 changes: 7 additions & 3 deletions include/sensors/tests/mock_hardware.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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++;
Expand All @@ -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; }

Expand Down
8 changes: 6 additions & 2 deletions pipettes/firmware/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
13 changes: 7 additions & 6 deletions pipettes/firmware/utility_configurations.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 <>
Expand Down
4 changes: 3 additions & 1 deletion pipettes/simulator/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -304,8 +304,10 @@ 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);
Expand Down
12 changes: 8 additions & 4 deletions sensors/tests/test_capacitive_sensor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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{};

Expand Down Expand Up @@ -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{};

Expand Down Expand Up @@ -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{};

Expand Down Expand Up @@ -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{};

Expand Down
3 changes: 2 additions & 1 deletion sensors/tests/test_pressure_driver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion sensors/tests/test_pressure_sensor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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{};
Expand Down
Loading
Loading