From cb57951b573fa8eea9154ee7fc4ec4fe328440c2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Niccol=C3=B2=20Betto?= <niccolo.betto@skywarder.eu>
Date: Wed, 19 Mar 2025 00:42:35 +0100
Subject: [PATCH] [ConRIG][ConRIGv2] Update button handling code to avoid
spurious disarming
---
src/ConRIG/Buttons/Buttons.cpp | 155 +++++---------------
src/ConRIG/Buttons/Buttons.h | 9 +-
src/ConRIG/Radio/Radio.cpp | 28 ++--
src/ConRIG/Radio/Radio.h | 10 +-
src/ConRIGv2/Buttons/Buttons.cpp | 233 ++++++-------------------------
src/ConRIGv2/Buttons/Buttons.h | 9 +-
src/ConRIGv2/Radio/Radio.cpp | 30 ++--
src/ConRIGv2/Radio/Radio.h | 8 +-
8 files changed, 127 insertions(+), 355 deletions(-)
diff --git a/src/ConRIG/Buttons/Buttons.cpp b/src/ConRIG/Buttons/Buttons.cpp
index 8390a9494..c4b5a98d1 100644
--- a/src/ConRIG/Buttons/Buttons.cpp
+++ b/src/ConRIG/Buttons/Buttons.cpp
@@ -32,11 +32,10 @@ using namespace miosix;
using namespace Boardcore;
using namespace ConRIG;
-Buttons::Buttons() : state() {}
-
bool Buttons::start()
{
- TaskScheduler& scheduler = getModule<BoardScheduler>()->getRadioScheduler();
+ TaskScheduler& scheduler =
+ getModule<BoardScheduler>()->getButtonsScheduler();
return scheduler.addTask([this]() { periodicStatusCheck(); },
Config::Buttons::BUTTON_SAMPLE_PERIOD) != 0;
@@ -44,131 +43,43 @@ bool Buttons::start()
mavlink_conrig_state_tc_t Buttons::getState() { return state; }
-void Buttons::resetState()
-{
- // Preserve the arm switch state
- auto armSwitch = state.arm_switch;
- state = {};
- state.arm_switch = armSwitch;
-}
-
void Buttons::periodicStatusCheck()
{
+#define CHECK_BUTTON(cond, btn) \
+ if (cond) \
+ { \
+ if (guard.btn > Config::Buttons::GUARD_THRESHOLD) \
+ { \
+ guard.btn = 0; \
+ state.btn = true; \
+ LOG_DEBUG(logger, #btn " button pressed"); \
+ } \
+ else \
+ { \
+ guard.btn++; \
+ } \
+ } \
+ else \
+ { \
+ guard.btn = 0; \
+ state.btn = false; \
+ }
+
state.arm_switch = btns::arm::value();
- if (!btns::ignition::value() && state.arm_switch)
- {
- if (guard > Config::Buttons::GUARD_THRESHOLD)
- {
- guard = 0;
- state.ignition_btn = true;
- LOG_DEBUG(logger, "Ignition button pressed");
- }
- else
- {
- guard++;
- }
- }
- else if (btns::n2o_filling::value())
- {
- if (guard > Config::Buttons::GUARD_THRESHOLD)
- {
- guard = 0;
- state.ox_filling_btn = true;
- LOG_DEBUG(logger, "ox filling button pressed");
- }
- else
- {
- guard++;
- }
- }
- else if (btns::n2o_release::value())
- {
- if (guard > Config::Buttons::GUARD_THRESHOLD)
- {
- guard = 0;
- state.ox_release_btn = true;
- LOG_DEBUG(logger, "ox release button pressed");
- }
- else
- {
- guard++;
- }
- }
- else if (btns::n2_release::value())
- {
- if (guard > Config::Buttons::GUARD_THRESHOLD)
- {
- guard = 0;
- state.n2_release_btn = true;
- LOG_DEBUG(logger, "n2 release button pressed");
- }
- else
- {
- guard++;
- }
- }
- else if (btns::n2o_venting::value())
- {
- if (guard > Config::Buttons::GUARD_THRESHOLD)
- {
- guard = 0;
- state.ox_venting_btn = true;
- LOG_DEBUG(logger, "ox venting button pressed");
- }
- else
- {
- guard++;
- }
- }
- else if (btns::n2_detach::value())
- {
- if (guard > Config::Buttons::GUARD_THRESHOLD)
- {
- guard = 0;
- state.n2_detach_btn = true;
- LOG_DEBUG(logger, "n2 detach button pressed");
- }
- else
- {
- guard++;
- }
- }
- else if (btns::n2_filling::value())
- {
- if (guard > Config::Buttons::GUARD_THRESHOLD)
- {
- guard = 0;
- state.n2_filling_btn = true;
- LOG_DEBUG(logger, "n2 filling button pressed");
- }
- else
- {
- guard++;
- }
- }
- else if (btns::nitrogen::value())
- {
- if (guard > Config::Buttons::GUARD_THRESHOLD)
- {
- guard = 0;
- state.nitrogen_btn = true;
- LOG_DEBUG(logger, "nitrogen button pressed");
- }
- else
- {
- guard++;
- }
- }
- else
- {
- // Reset all the states and guard
- guard = 0;
- resetState();
- }
+ CHECK_BUTTON(!btns::ignition::value() && state.arm_switch, ignition_btn);
+ CHECK_BUTTON(btns::n2o_filling::value(), ox_filling_btn);
+ CHECK_BUTTON(btns::n2o_release::value(), ox_release_btn);
+ CHECK_BUTTON(btns::n2_release::value(), n2_release_btn);
+ CHECK_BUTTON(btns::n2o_venting::value(), ox_venting_btn);
+ CHECK_BUTTON(btns::n2_detach::value(), n2_detach_btn);
+ CHECK_BUTTON(btns::n2_filling::value(), n2_filling_btn);
+ CHECK_BUTTON(btns::nitrogen::value(), nitrogen_btn);
+
+#undef CHECK_BUTTON
// Set the internal button state in Radio module
- getModule<Radio>()->setButtonsState(state);
+ getModule<Radio>()->updateButtonState(state);
}
void Buttons::enableIgnition() { ui::armedLed::high(); }
diff --git a/src/ConRIG/Buttons/Buttons.h b/src/ConRIG/Buttons/Buttons.h
index ecd160f2f..2d6fff4ae 100644
--- a/src/ConRIG/Buttons/Buttons.h
+++ b/src/ConRIG/Buttons/Buttons.h
@@ -36,8 +36,6 @@ class Radio;
class Buttons : public Boardcore::InjectableWithDeps<BoardScheduler, Radio>
{
public:
- Buttons();
-
[[nodiscard]] bool start();
mavlink_conrig_state_tc_t getState();
@@ -46,14 +44,11 @@ public:
void disableIgnition();
private:
- void resetState();
-
void periodicStatusCheck();
- mavlink_conrig_state_tc_t state;
-
+ mavlink_conrig_state_tc_t state{};
// Counter guard to avoid spurious triggers
- uint8_t guard = 0;
+ mavlink_conrig_state_tc_t guard{};
Boardcore::PrintLogger logger = Boardcore::Logging::getLogger("buttons");
};
diff --git a/src/ConRIG/Radio/Radio.cpp b/src/ConRIG/Radio/Radio.cpp
index f6b4807cb..cb88b002b 100644
--- a/src/ConRIG/Radio/Radio.cpp
+++ b/src/ConRIG/Radio/Radio.cpp
@@ -80,9 +80,8 @@ void Radio::handleMessage(const mavlink_message_t& msg)
// we assume this ack is about the last sent message
if (id == MAVLINK_MSG_ID_CONRIG_STATE_TC)
{
- Lock<FastMutex> lock{buttonsMutex};
// Reset the internal button state
- buttonState = {};
+ resetButtonState();
}
break;
@@ -156,11 +155,11 @@ void Radio::loopBuzzer()
}
}
-void Radio::setButtonsState(mavlink_conrig_state_tc_t state)
+void Radio::updateButtonState(const mavlink_conrig_state_tc_t& state)
{
Lock<FastMutex> lock{buttonsMutex};
- // The OR operator is introduced to make sure that the receiver
- // understood the command
+ // Merge the new state with the old one, an extra-dumb way to ensure
+ // that we don't lose any button pressess
buttonState.ox_filling_btn |= state.ox_filling_btn;
buttonState.ox_release_btn |= state.ox_release_btn;
buttonState.n2_filling_btn |= state.n2_filling_btn;
@@ -170,11 +169,24 @@ void Radio::setButtonsState(mavlink_conrig_state_tc_t state)
buttonState.nitrogen_btn |= state.nitrogen_btn;
buttonState.ox_detach_btn |= state.ox_detach_btn;
buttonState.n2_quenching_btn |= state.n2_quenching_btn;
- buttonState.n2_3way_btn |= state.n2_3way_btn;
buttonState.tars3_btn |= state.tars3_btn;
buttonState.tars3m_btn |= state.tars3m_btn;
buttonState.ignition_btn |= state.ignition_btn;
- buttonState.arm_switch |= state.arm_switch;
+
+ // Don't merge lever states
+ buttonState.n2_3way_btn = state.n2_3way_btn;
+ buttonState.arm_switch = state.arm_switch;
+}
+
+void Radio::resetButtonState()
+{
+ Lock<FastMutex> lock{buttonsMutex};
+ // Save and restore lever states
+ auto n2_3way_btn = buttonState.n2_3way_btn;
+ auto arm_switch = buttonState.arm_switch;
+ buttonState = {};
+ buttonState.n2_3way_btn = n2_3way_btn;
+ buttonState.arm_switch = arm_switch;
}
bool Radio::start()
@@ -229,5 +241,3 @@ bool Radio::start()
}
MavlinkStatus Radio::getMavlinkStatus() { return mavDriver->getStatus(); }
-
-Radio::Radio() : buttonState() {}
diff --git a/src/ConRIG/Radio/Radio.h b/src/ConRIG/Radio/Radio.h
index f9d5edb29..949df5f99 100644
--- a/src/ConRIG/Radio/Radio.h
+++ b/src/ConRIG/Radio/Radio.h
@@ -49,13 +49,11 @@ class Radio : public Boardcore::InjectableWithDeps<Buses, BoardScheduler,
Buttons, Serial>
{
public:
- Radio();
-
[[nodiscard]] bool start();
Boardcore::MavlinkStatus getMavlinkStatus();
- void setButtonsState(mavlink_conrig_state_tc_t state);
+ void updateButtonState(const mavlink_conrig_state_tc_t& state);
bool enqueueMessage(const mavlink_message_t& msg);
@@ -64,6 +62,8 @@ private:
void loopBuzzer();
void handleMessage(const mavlink_message_t& msg);
+ void resetButtonState();
+
std::unique_ptr<Boardcore::SX1278Lora> radio;
std::unique_ptr<MavDriver> mavDriver;
@@ -72,10 +72,10 @@ private:
messageQueue;
miosix::FastMutex queueMutex;
- miosix::FastMutex buttonsMutex;
// Button internal state
- mavlink_conrig_state_tc_t buttonState;
+ miosix::FastMutex buttonsMutex;
+ mavlink_conrig_state_tc_t buttonState{};
std::thread buzzerLooper;
diff --git a/src/ConRIGv2/Buttons/Buttons.cpp b/src/ConRIGv2/Buttons/Buttons.cpp
index e7056ab79..f30478643 100644
--- a/src/ConRIGv2/Buttons/Buttons.cpp
+++ b/src/ConRIGv2/Buttons/Buttons.cpp
@@ -1,5 +1,5 @@
/* Copyright (c) 2025 Skyward Experimental Rocketry
- * Author: Ettore Pane
+ * Authors: Ettore Pane, Niccolò Betto
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
@@ -32,15 +32,10 @@ using namespace miosix;
using namespace Boardcore;
using namespace ConRIGv2;
-Buttons::Buttons()
-{
- resetState();
- state.arm_switch = false;
-}
-
bool Buttons::start()
{
- TaskScheduler& scheduler = getModule<BoardScheduler>()->getRadioScheduler();
+ TaskScheduler& scheduler =
+ getModule<BoardScheduler>()->getButtonsScheduler();
return scheduler.addTask([this]() { periodicStatusCheck(); },
Config::Buttons::BUTTON_SAMPLE_PERIOD) != 0;
@@ -48,196 +43,48 @@ bool Buttons::start()
mavlink_conrig_state_tc_t Buttons::getState() { return state; }
-void Buttons::resetState()
-{
- // Preserve the arm switch state
- auto armSwitch = state.arm_switch;
- state = {};
- state.arm_switch = armSwitch;
-}
-
void Buttons::periodicStatusCheck()
{
- state.arm_switch = btns::arm::value();
-
- if (!btns::ignition::value() && state.arm_switch)
- {
- if (guard > Config::Buttons::GUARD_THRESHOLD)
- {
- guard = 0;
- state.ignition_btn = true;
- LOG_DEBUG(logger, "Ignition button pressed");
- }
- else
- {
- guard++;
- }
- }
- else if (btns::ox_filling::value())
- {
- if (guard > Config::Buttons::GUARD_THRESHOLD)
- {
- guard = 0;
- state.ox_filling_btn = true;
- LOG_DEBUG(logger, "Ox filling button pressed");
- }
- else
- {
- guard++;
- }
- }
- else if (btns::ox_release::value())
- {
- if (guard > Config::Buttons::GUARD_THRESHOLD)
- {
- guard = 0;
- state.ox_release_btn = true;
- LOG_DEBUG(logger, "Ox release button pressed");
- }
- else
- {
- guard++;
- }
- }
- else if (btns::ox_detach::value())
- {
- if (guard > Config::Buttons::GUARD_THRESHOLD)
- {
- guard = 0;
- state.ox_detach_btn = true;
- LOG_DEBUG(logger, "Ox detach button pressed");
- }
- else
- {
- guard++;
- }
- }
- else if (btns::n2_3way::value())
- {
- if (guard > Config::Buttons::GUARD_THRESHOLD)
- {
- guard = 0;
- state.ox_detach_btn = true;
- LOG_DEBUG(logger, "n2 3way button pressed");
- }
- else
- {
- guard++;
- }
- }
- else if (btns::n2_filling::value())
- {
- if (guard > Config::Buttons::GUARD_THRESHOLD)
- {
- guard = 0;
- state.n2_filling_btn = true;
- LOG_DEBUG(logger, "N2 filling button pressed");
- }
- else
- {
- guard++;
- }
- }
- else if (btns::n2_release::value())
- {
- if (guard > Config::Buttons::GUARD_THRESHOLD)
- {
- guard = 0;
- state.n2_release_btn = true;
- LOG_DEBUG(logger, "N2 release button pressed");
- }
- else
- {
- guard++;
- }
- }
- else if (btns::n2_detach::value())
- {
- if (guard > Config::Buttons::GUARD_THRESHOLD)
- {
- guard = 0;
- state.n2_detach_btn = true;
- LOG_DEBUG(logger, "N2 detach button pressed");
- }
- else
- {
- guard++;
- }
- }
- else if (btns::nitrogen::value())
- {
- if (guard > Config::Buttons::GUARD_THRESHOLD)
- {
- guard = 0;
- state.nitrogen_btn = true;
- LOG_DEBUG(logger, "Nitrogen button pressed");
- }
- else
- {
- guard++;
- }
- }
- else if (btns::ox_venting::value())
- {
- if (guard > Config::Buttons::GUARD_THRESHOLD)
- {
- guard = 0;
- state.ox_venting_btn = true;
- LOG_DEBUG(logger, "Ox venting button pressed");
- }
- else
- {
- guard++;
- }
- }
- else if (btns::n2_quenching::value())
- {
- if (guard > Config::Buttons::GUARD_THRESHOLD)
- {
- guard = 0;
- state.n2_quenching_btn = true;
- LOG_DEBUG(logger, "N2 quenching button pressed");
- }
- else
- {
- guard++;
- }
- }
- else if (btns::tars3::value())
- {
- if (guard > Config::Buttons::GUARD_THRESHOLD)
- {
- guard = 0;
- state.tars3_btn = true;
- LOG_DEBUG(logger, "Tars3 button pressed");
- }
- else
- {
- guard++;
- }
- }
- else if (btns::tars3m::value())
- {
- if (guard > Config::Buttons::GUARD_THRESHOLD)
- {
- guard = 0;
- state.tars3m_btn = true;
- LOG_DEBUG(logger, "Tars3m button pressed");
- }
- else
- {
- guard++;
- }
- }
- else
- {
- // Reset all the states and guard
- guard = 0;
- resetState();
+#define CHECK_BUTTON(cond, btn) \
+ if (cond) \
+ { \
+ if (guard.btn > Config::Buttons::GUARD_THRESHOLD) \
+ { \
+ guard.btn = 0; \
+ state.btn = true; \
+ LOG_DEBUG(logger, #btn " button pressed"); \
+ } \
+ else \
+ { \
+ guard.btn++; \
+ } \
+ } \
+ else \
+ { \
+ guard.btn = 0; \
+ state.btn = false; \
}
+ state.arm_switch = btns::arm::value();
+ state.n2_3way_btn = btns::n2_3way::value();
+
+ CHECK_BUTTON(!btns::ignition::value() && state.arm_switch, ignition_btn);
+ CHECK_BUTTON(btns::ox_filling::value(), ox_filling_btn);
+ CHECK_BUTTON(btns::ox_release::value(), ox_release_btn);
+ CHECK_BUTTON(btns::ox_detach::value(), ox_detach_btn);
+ CHECK_BUTTON(btns::ox_venting::value(), ox_venting_btn);
+ CHECK_BUTTON(btns::n2_filling::value(), n2_filling_btn);
+ CHECK_BUTTON(btns::n2_release::value(), n2_release_btn);
+ CHECK_BUTTON(btns::n2_detach::value(), n2_detach_btn);
+ CHECK_BUTTON(btns::n2_quenching::value(), n2_quenching_btn);
+ CHECK_BUTTON(btns::nitrogen::value(), nitrogen_btn);
+ CHECK_BUTTON(btns::tars3::value(), tars3_btn);
+ CHECK_BUTTON(btns::tars3m::value(), tars3m_btn);
+
+#undef CHECK_BUTTON
+
// Set the internal button state in Radio module
- getModule<Radio>()->setButtonsState(state);
+ getModule<Radio>()->updateButtonState(state);
}
void Buttons::enableIgnition() { ui::armedLed::high(); }
diff --git a/src/ConRIGv2/Buttons/Buttons.h b/src/ConRIGv2/Buttons/Buttons.h
index 5d2c26d0e..2c4d54408 100644
--- a/src/ConRIGv2/Buttons/Buttons.h
+++ b/src/ConRIGv2/Buttons/Buttons.h
@@ -36,8 +36,6 @@ class Radio;
class Buttons : public Boardcore::InjectableWithDeps<BoardScheduler, Radio>
{
public:
- Buttons();
-
[[nodiscard]] bool start();
mavlink_conrig_state_tc_t getState();
@@ -46,14 +44,11 @@ public:
void disableIgnition();
private:
- void resetState();
-
void periodicStatusCheck();
- mavlink_conrig_state_tc_t state;
-
+ mavlink_conrig_state_tc_t state{};
// Counter guard to avoid spurious triggers
- uint8_t guard = 0;
+ mavlink_conrig_state_tc_t guard{};
Boardcore::PrintLogger logger = Boardcore::Logging::getLogger("buttons");
};
diff --git a/src/ConRIGv2/Radio/Radio.cpp b/src/ConRIGv2/Radio/Radio.cpp
index 8e7ebb843..328df566a 100644
--- a/src/ConRIGv2/Radio/Radio.cpp
+++ b/src/ConRIGv2/Radio/Radio.cpp
@@ -1,5 +1,5 @@
/* Copyright (c) 2025 Skyward Experimental Rocketry
- * Author: Ettore Pane
+ * Authors: Ettore Pane, Niccolò Betto
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
@@ -80,9 +80,8 @@ void Radio::handleMessage(const mavlink_message_t& msg)
// we assume this ack is about the last sent message
if (id == MAVLINK_MSG_ID_CONRIG_STATE_TC)
{
- Lock<FastMutex> lock{buttonsMutex};
// Reset the internal button state
- buttonState = {};
+ resetButtonState();
}
break;
@@ -164,11 +163,11 @@ void Radio::buzzerTask()
}
}
-void Radio::setButtonsState(const mavlink_conrig_state_tc_t& state)
+void Radio::updateButtonState(const mavlink_conrig_state_tc_t& state)
{
Lock<FastMutex> lock{buttonsMutex};
- // The OR operator is introduced to make sure that the receiver
- // understood the command
+ // Merge the new state with the old one, an extra-dumb way to ensure
+ // that we don't lose any button pressess
buttonState.ox_filling_btn |= state.ox_filling_btn;
buttonState.ox_release_btn |= state.ox_release_btn;
buttonState.n2_filling_btn |= state.n2_filling_btn;
@@ -178,11 +177,24 @@ void Radio::setButtonsState(const mavlink_conrig_state_tc_t& state)
buttonState.nitrogen_btn |= state.nitrogen_btn;
buttonState.ox_detach_btn |= state.ox_detach_btn;
buttonState.n2_quenching_btn |= state.n2_quenching_btn;
- buttonState.n2_3way_btn |= state.n2_3way_btn;
buttonState.tars3_btn |= state.tars3_btn;
buttonState.tars3m_btn |= state.tars3m_btn;
buttonState.ignition_btn |= state.ignition_btn;
- buttonState.arm_switch |= state.arm_switch;
+
+ // Don't merge lever states
+ buttonState.n2_3way_btn = state.n2_3way_btn;
+ buttonState.arm_switch = state.arm_switch;
+}
+
+void Radio::resetButtonState()
+{
+ Lock<FastMutex> lock{buttonsMutex};
+ // Save and restore lever states
+ auto n2_3way_btn = buttonState.n2_3way_btn;
+ auto arm_switch = buttonState.arm_switch;
+ buttonState = {};
+ buttonState.n2_3way_btn = n2_3way_btn;
+ buttonState.arm_switch = arm_switch;
}
bool Radio::start()
@@ -239,7 +251,7 @@ bool Radio::start()
MavlinkStatus Radio::getMavlinkStatus() { return mavDriver->getStatus(); }
-Radio::Radio() : buzzer(MIOSIX_BUZZER_TIM, 523), buttonState()
+Radio::Radio() : buzzer(MIOSIX_BUZZER_TIM, 523)
{
buzzer.setDutyCycle(TimerUtils::Channel::MIOSIX_BUZZER_CHANNEL, 0.5);
}
diff --git a/src/ConRIGv2/Radio/Radio.h b/src/ConRIGv2/Radio/Radio.h
index 399eb42fb..9d8c8bb47 100644
--- a/src/ConRIGv2/Radio/Radio.h
+++ b/src/ConRIGv2/Radio/Radio.h
@@ -56,7 +56,7 @@ public:
Boardcore::MavlinkStatus getMavlinkStatus();
- void setButtonsState(const mavlink_conrig_state_tc_t& state);
+ void updateButtonState(const mavlink_conrig_state_tc_t& state);
bool enqueueMessage(const mavlink_message_t& msg);
@@ -65,6 +65,8 @@ private:
void buzzerTask();
void handleMessage(const mavlink_message_t& msg);
+ void resetButtonState();
+
std::unique_ptr<Boardcore::SX1278Lora> radio;
std::unique_ptr<MavDriver> mavDriver;
@@ -81,10 +83,10 @@ private:
messageQueue;
miosix::FastMutex queueMutex;
- miosix::FastMutex buttonsMutex;
// Button internal state
- mavlink_conrig_state_tc_t buttonState;
+ miosix::FastMutex buttonsMutex;
+ mavlink_conrig_state_tc_t buttonState{};
std::atomic<uint8_t> messagesReceived{0};
std::atomic<bool> isArmed{false};
--
GitLab