From fce219555840ce37de3b0a4a75e6dce33b20c61d Mon Sep 17 00:00:00 2001 From: "D.R.racer" Date: Fri, 21 May 2021 12:34:11 +0200 Subject: [PATCH 1/7] Interface module for driving LEDs + start shaping up main.cpp + make the usage of namespaces and class names more consistent throughout the whole project + refactor related unit tests accordingly --- CMakeLists.txt | 2 +- src/hal/adc.h | 4 +- src/hal/avr/cpu.cpp | 2 +- src/hal/cpu.h | 4 +- src/main.cpp | 167 +++++--- src/modules/buttons.cpp | 119 +++--- src/modules/buttons.h | 100 ++--- src/modules/leds.cpp | 50 +++ src/modules/leds.h | 118 +++++- src/modules/protocol.cpp | 370 +++++++++--------- src/modules/protocol.h | 220 +++++------ tests/unit/modules/buttons/stub_adc.cpp | 4 +- tests/unit/modules/buttons/stub_adc.h | 4 +- tests/unit/modules/buttons/test_buttons.cpp | 50 +-- tests/unit/modules/protocol/test_protocol.cpp | 86 ++-- 15 files changed, 772 insertions(+), 528 deletions(-) create mode 100644 src/modules/leds.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 326a896..239ae85 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -184,7 +184,7 @@ target_include_directories(firmware PRIVATE include src) target_compile_options(firmware PRIVATE -Wdouble-promotion) target_sources( firmware PRIVATE src/main.cpp src/hal/avr/cpu.cpp src/hal/avr/usart.cpp src/modules/protocol.cpp - src/modules/buttons.cpp + src/modules/buttons.cpp src/modules/leds.cpp ) set_property( diff --git a/src/hal/adc.h b/src/hal/adc.h index 61a0f26..cd9cef6 100644 --- a/src/hal/adc.h +++ b/src/hal/adc.h @@ -4,10 +4,10 @@ /// Hardware Abstraction Layer for the ADC's namespace hal { -namespace ADC { +namespace adc { /// ADC access routines uint16_t ReadADC(uint8_t adc); -} // namespace ADC +} // namespace adc } // namespace hal diff --git a/src/hal/avr/cpu.cpp b/src/hal/avr/cpu.cpp index de23c60..991342c 100644 --- a/src/hal/avr/cpu.cpp +++ b/src/hal/avr/cpu.cpp @@ -1,7 +1,7 @@ #include "../cpu.h" namespace hal { -namespace CPU { +namespace cpu { void Init() { } diff --git a/src/hal/cpu.h b/src/hal/cpu.h index 1f7e2c0..880cc79 100644 --- a/src/hal/cpu.h +++ b/src/hal/cpu.h @@ -3,10 +3,10 @@ /// Hardware Abstraction Layer for the CPU namespace hal { -namespace CPU { +namespace cpu { /// CPU init routines (not really necessary for the AVR) void Init(); -} // namespace CPU +} // namespace cpu } // namespace hal diff --git a/src/main.cpp b/src/main.cpp index 9ccaf8a..cb72bca 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1,65 +1,135 @@ -#include "logic/mm_control.h" +#include "hal/cpu.h" +#include "hal/adc.h" #include "hal/gpio.h" #include "hal/spi.h" #include "hal/usart.h" + #include "pins.h" #include +#include "modules/buttons.h" +#include "modules/leds.h" +#include "modules/protocol.h" + +#include "logic/mm_control.h" + +static hal::UART uart; + +static modules::protocol::Protocol protocol; +static modules::buttons::Buttons buttons; +static modules::leds::LEDs leds; + +// examples and test code shall be located here +void TmpPlayground() { + using namespace hal; + + // SPI example + gpio::Init(gpio::GPIO_pin(GPIOC, 6), gpio::GPIO_InitTypeDef(gpio::Mode::output, gpio::Level::high)); + uint8_t dat[5]; + gpio::WritePin(gpio::GPIO_pin(GPIOC, 6), gpio::Level::low); + spi::TxRx(SPI0, 0x01); + spi::TxRx(SPI0, 0x00); + spi::TxRx(SPI0, 0x00); + spi::TxRx(SPI0, 0x00); + spi::TxRx(SPI0, 0x00); + gpio::WritePin(gpio::GPIO_pin(GPIOC, 6), gpio::Level::high); + gpio::WritePin(gpio::GPIO_pin(GPIOC, 6), gpio::Level::low); + dat[0] = spi::TxRx(SPI0, 0x00); + dat[1] = spi::TxRx(SPI0, 0x00); + dat[2] = spi::TxRx(SPI0, 0x00); + dat[3] = spi::TxRx(SPI0, 0x00); + dat[4] = spi::TxRx(SPI0, 0x00); + gpio::WritePin(gpio::GPIO_pin(GPIOC, 6), gpio::Level::high); + (void)dat; + + // using namespace hal::gpio; + // WritePin(GPIO_pin(GPIOB, 5), Level::low); + // TogglePin(GPIO_pin(GPIOB, 6)); + // if (hal::gpio::ReadPin(GPIO_pin(GPIOB, 7)) == hal::gpio::Level::low) + // break; + + sei(); + usart1.puts("1234567890\n"); + usart1.puts("1234567890\n"); + usart1.puts("1234567890\n"); + usart1.puts("1234567890\n"); + usart1.puts("1234567890\n"); + usart1.puts("1234567890\n"); + usart1.puts("1234567890\n"); + usart1.puts("1234567890\n"); + usart1.puts("1234567890\n"); + usart1.puts("1234567890\n"); + usart1.puts("1234567890\n"); +} + /// One-time setup of HW and SW components /// Called before entering the loop() function +/// Green LEDs signalize the progress of initialization. If anything goes wrong we shall turn on a red LED void setup() { using namespace hal; - // spi::SPI_InitTypeDef spi_conf = { - // .miso_pin = gpio::GPIO_pin(TMC2130_SPI_MISO_PIN), - // .mosi_pin = gpio::GPIO_pin(TMC2130_SPI_MOSI_PIN), - // .sck_pin = gpio::GPIO_pin(TMC2130_SPI_SCK_PIN), - // .ss_pin = gpio::GPIO_pin(TMC2130_SPI_SS_PIN), - // .prescaler = 2, //4mhz - // .cpha = 1, - // .cpol = 1, - // }; - // spi::Init(SPI0, &spi_conf); + cpu::Init(); - // // SPI example - // gpio::Init(gpio::GPIO_pin(GPIOC, 6), gpio::GPIO_InitTypeDef(gpio::Mode::output, gpio::Level::high)); - // uint8_t dat[5]; - // gpio::WritePin(gpio::GPIO_pin(GPIOC, 6), gpio::Level::low); - // spi::TxRx(SPI0, 0x01); - // spi::TxRx(SPI0, 0x00); - // spi::TxRx(SPI0, 0x00); - // spi::TxRx(SPI0, 0x00); - // spi::TxRx(SPI0, 0x00); - // gpio::WritePin(gpio::GPIO_pin(GPIOC, 6), gpio::Level::high); - // gpio::WritePin(gpio::GPIO_pin(GPIOC, 6), gpio::Level::low); - // dat[0] = spi::TxRx(SPI0, 0x00); - // dat[1] = spi::TxRx(SPI0, 0x00); - // dat[2] = spi::TxRx(SPI0, 0x00); - // dat[3] = spi::TxRx(SPI0, 0x00); - // dat[4] = spi::TxRx(SPI0, 0x00); - // gpio::WritePin(gpio::GPIO_pin(GPIOC, 6), gpio::Level::high); - // (void)dat; + // shr::Init() + leds.SetMode(4, false, modules::leds::Mode::blink0); + // shr::Send(leds.Step(0)); - USART::USART_InitTypeDef usart_conf = { + // @@TODO if the shift register doesn't work we really can't signalize anything, only internal variables will be accessible if the UART works + + USART::USART_InitTypeDef usart_conf = { .rx_pin = gpio::GPIO_pin(GPIOD, 2), .tx_pin = gpio::GPIO_pin(GPIOD, 3), .baudrate = 115200, - }; - usart1.Init(&usart_conf); - sei(); - usart1.puts("1234567890\n"); - usart1.puts("1234567890\n"); - usart1.puts("1234567890\n"); - usart1.puts("1234567890\n"); - usart1.puts("1234567890\n"); - usart1.puts("1234567890\n"); - usart1.puts("1234567890\n"); - usart1.puts("1234567890\n"); - usart1.puts("1234567890\n"); - usart1.puts("1234567890\n"); - usart1.puts("1234567890\n"); - // usart1.Flush(); + + leds.SetMode(3, false, modules::leds::Mode::on); + // shr::Send(leds.Step(0)); + + // @@TODO if both shift register and the UART are dead, we are sitting ducks :( + + spi::SPI_InitTypeDef spi_conf = { + .miso_pin = gpio::GPIO_pin(TMC2130_SPI_MISO_PIN), + .mosi_pin = gpio::GPIO_pin(TMC2130_SPI_MOSI_PIN), + .sck_pin = gpio::GPIO_pin(TMC2130_SPI_SCK_PIN), + .ss_pin = gpio::GPIO_pin(TMC2130_SPI_SS_PIN), + .prescaler = 2, //4mhz + .cpha = 1, + .cpol = 1, + }; + spi::Init(SPI0, &spi_conf); + leds.SetMode(2, false, modules::leds::Mode::on); + //shr::Send(leds.Step(0)); + + // tmc::Init() + leds.SetMode(1, false, modules::leds::Mode::on); + //shr::Send(leds.Step(0)); + + // adc::Init(); + leds.SetMode(0, false, modules::leds::Mode::on); + //shr::Send(leds.Step(0)); +} + +void ProcessRequestMsg(const modules::protocol::RequestMsg &rq) { +} + +/// @returns true if a request was successfully finished +bool CheckMsgs() { + using mpd = modules::protocol::DecodeStatus; + while (!uart.ReadEmpty()) { + switch (protocol.DecodeRequest(uart.Read())) { + case mpd::MessageCompleted: + // process the input message + return true; + break; + case mpd::NeedMoreData: + // just continue reading + break; + case mpd::Error: + // what shall we do? Start some watchdog? + break; + } + } + return false; } /// Main loop of the firmware @@ -77,13 +147,16 @@ void setup() { /// The idea behind the Step* routines is to keep each automaton non-blocking allowing for some “concurrency”. /// Some FW components will leverage ISR to do their stuff (UART, motor stepping?, etc.) void loop() { + if (CheckMsgs()) { + ProcessRequestMsg(protocol.GetRequestMsg()); + } + buttons.Step(hal::adc::ReadADC(0)); + // shr.Send(leds.Step(0)); } int main() { setup(); for (;;) { - if (!usart1.ReadEmpty()) - usart1.Write(usart1.Read()); loop(); } return 0; diff --git a/src/modules/buttons.cpp b/src/modules/buttons.cpp index 70d726c..7ce76e1 100644 --- a/src/modules/buttons.cpp +++ b/src/modules/buttons.cpp @@ -1,72 +1,71 @@ #include "buttons.h" -#include "../hal/adc.h" namespace modules { +namespace buttons { -uint16_t Buttons::tmpTiming = 0; + uint16_t Buttons::tmpTiming = 0; -// original idea from: https://www.eeweb.com/debouncing-push-buttons-using-a-state-machine-approach -void Button::Step(uint16_t time, bool press) { - switch (f.state) { - case State::Waiting: - if (press) { - f.state = State::Detected; - timeLastChange = time; - f.tmp = press; - } - break; - case State::Detected: - if (f.tmp == press) { - if (time - timeLastChange > debounce) { - f.state = State::WaitForRelease; + // original idea from: https://www.eeweb.com/debouncing-push-buttons-using-a-state-machine-approach + void Button::Step(uint16_t time, bool press) { + switch (f.state) { + case State::Waiting: + if (press) { + f.state = State::Detected; + timeLastChange = time; + f.tmp = press; } - } else { + break; + case State::Detected: + if (f.tmp == press) { + if (time - timeLastChange > debounce) { + f.state = State::WaitForRelease; + } + } else { + f.state = State::Waiting; + } + break; + case State::WaitForRelease: + if (!press) { + f.state = State::Update; + } + break; + case State::Update: f.state = State::Waiting; + timeLastChange = time; + f.tmp = false; + break; + default: + f.state = State::Waiting; + timeLastChange = time; + f.tmp = false; } - break; - case State::WaitForRelease: - if (!press) { - f.state = State::Update; + } + + int8_t Buttons::Sample(uint16_t rawADC) { + // decode 3 buttons' levels from one ADC + // Button 1 - 0 + // Button 2 - 344 + // Button 3 - 516 + // Doesn't handle multiple pressed buttons at once + + if (rawADC < 10) + return 0; + else if (rawADC > 320 && rawADC < 360) + return 1; + else if (rawADC > 500 && rawADC < 530) + return 2; + return -1; + } + + void Buttons::Step(uint16_t rawADC) { + // @@TODO temporary timing + ++tmpTiming; + int8_t currentState = Sample(rawADC); + for (uint_fast8_t b = 0; b < N; ++b) { + // this button was pressed if b == currentState, released otherwise + buttons[b].Step(tmpTiming, b == currentState); } - break; - case State::Update: - f.state = State::Waiting; - timeLastChange = time; - f.tmp = false; - break; - default: - f.state = State::Waiting; - timeLastChange = time; - f.tmp = false; } -} - -int8_t Buttons::Sample() { - // decode 3 buttons' levels from one ADC - uint16_t raw = hal::ADC::ReadADC(0); - - // Button 1 - 0 - // Button 2 - 344 - // Button 3 - 516 - // Doesn't handle multiple pressed buttons at once - - if (raw < 10) - return 0; - else if (raw > 320 && raw < 360) - return 1; - else if (raw > 500 && raw < 530) - return 2; - return -1; -} - -void Buttons::Step() { - // @@TODO temporary timing - ++tmpTiming; - int8_t currentState = Sample(); - for (uint_fast8_t b = 0; b < N; ++b) { - // this button was pressed if b == currentState, released otherwise - buttons[b].Step(tmpTiming, b == currentState); - } -} +} // namespace buttons } // namespace modules diff --git a/src/modules/buttons.h b/src/modules/buttons.h index c3a3ea3..d20260e 100644 --- a/src/modules/buttons.h +++ b/src/modules/buttons.h @@ -7,67 +7,69 @@ /// This layer should contain debouncing of buttons and their logical interpretation namespace modules { +namespace buttons { -struct Button { - inline constexpr Button() - : timeLastChange(0) {} + struct Button { + inline constexpr Button() + : timeLastChange(0) {} - /// @returns true if button is currently considered as pressed - inline bool Pressed() const { return f.state == State::WaitForRelease; } + /// @returns true if button is currently considered as pressed + inline bool Pressed() const { return f.state == State::WaitForRelease; } - /// State machine stepping routine - void Step(uint16_t time, bool press); + /// State machine stepping routine + void Step(uint16_t time, bool press); -private: - /// time interval for debouncing @@TODO specify units - constexpr static const uint16_t debounce = 100; + private: + /// time interval for debouncing @@TODO specify units + constexpr static const uint16_t debounce = 100; - /// States of the debouncing automaton - /// Intentionally not modeled as an enum class - /// as it would impose additional casts which do not play well with the struct Flags - /// and would make the code less readable - enum State { Waiting = 0, - Detected, - WaitForRelease, - Update }; + /// States of the debouncing automaton + /// Intentionally not modeled as an enum class + /// as it would impose additional casts which do not play well with the struct Flags + /// and would make the code less readable + enum State { Waiting = 0, + Detected, + WaitForRelease, + Update }; - /// The sole purpose of this data struct is to save RAM by compressing several flags into one byte on the AVR - struct Flags { - uint8_t state : 2; ///< state of the button - uint8_t tmp : 1; ///< temporary state of button before the debouncing state machine finishes - inline constexpr Flags() - : state(State::Waiting) - , tmp(false) {} + /// The sole purpose of this data struct is to save RAM by compressing several flags into one byte on the AVR + struct Flags { + uint8_t state : 2; ///< state of the button + uint8_t tmp : 1; ///< temporary state of button before the debouncing state machine finishes + inline constexpr Flags() + : state(State::Waiting) + , tmp(false) {} + }; + + /// Flags and state of the debouncing automaton + Flags f; + + /// Timestamp of the last change of ADC state for this button + uint16_t timeLastChange; }; - /// Flags and state of the debouncing automaton - Flags f; + class Buttons { + constexpr static const uint8_t N = 3; ///< number of buttons currently supported + constexpr static const uint8_t adc = 1; ///< ADC index - will be some define or other constant later on + static uint16_t tmpTiming; ///< subject to removal when we have timers implemented - now used for the unit tests - /// Timestamp of the last change of ADC state for this button - uint16_t timeLastChange; -}; + public: + inline constexpr Buttons() = default; -class Buttons { - constexpr static const uint8_t N = 3; ///< number of buttons currently supported - constexpr static const uint8_t adc = 1; ///< ADC index - will be some define or other constant later on - static uint16_t tmpTiming; ///< subject to removal when we have timers implemented - now used for the unit tests + /// State machine step - reads the ADC, processes debouncing, updates states of individual buttons + void Step(uint16_t rawADC); -public: - inline constexpr Buttons() = default; + /// @return true if button at index is pressed + /// @@TODO add range checking if necessary + inline bool ButtonPressed(uint8_t index) const { return buttons[index].Pressed(); } - /// State machine step - reads the ADC, processes debouncing, updates states of individual buttons - void Step(); + private: + Button buttons[N]; - /// @return true if button at index is pressed - /// @@TODO add range checking if necessary - inline bool ButtonPressed(uint8_t index) const { return buttons[index].Pressed(); } - -private: - Button buttons[N]; - - /// Call to the ADC and decode its output into a button index - /// @returns index of the button pressed or -1 in case no button is pressed - static int8_t Sample(); -}; + /// Call to the ADC and decode its output into a button index + /// @returns index of the button pressed or -1 in case no button is pressed + static int8_t Sample(uint16_t rawADC); + }; +} // namespace buttons } // namespace modules diff --git a/src/modules/leds.cpp b/src/modules/leds.cpp new file mode 100644 index 0000000..c994d42 --- /dev/null +++ b/src/modules/leds.cpp @@ -0,0 +1,50 @@ +#include "leds.h" + +namespace modules { +namespace leds { + + void LED::SetMode(Mode mode) { + state.mode = mode; + // set initial state of LEDs correctly - transition from one mode to another + switch (state.mode) { + case Mode::blink1: + case Mode::off: + state.on = 0; + break; + + case Mode::blink0: + case Mode::on: + state.on = 1; + break; + default: + break; + } + } + + bool LED::Step(bool oddPeriod) { + switch (state.mode) { + // on and off don't change while stepping + case Mode::blink0: + state.on = oddPeriod; + break; + case Mode::blink1: + state.on = !oddPeriod; + break; + default: // do nothing + break; + } + } + + uint16_t LEDs::Step(uint8_t delta_ms) { + ms += delta_ms; + bool oddPeriod = ((ms / 1000U) & 0x01U) != 0; + uint16_t result = 0; + for (uint8_t i = 0; i < ledPairs * 2; ++i) { + result <<= 1; + result |= leds[i].Step(oddPeriod); + } + return result; + } + +} // namespace leds +} // namespace modules diff --git a/src/modules/leds.h b/src/modules/leds.h index 54041c7..d3168b4 100644 --- a/src/modules/leds.h +++ b/src/modules/leds.h @@ -1,3 +1,119 @@ #pragma once -/// @@TODO @leptun design some nice API ;) +#include + +/// We have 5 pairs of LEDs +/// In each pair there is a green and a red LED +/// +/// A typical scenario in the past was visualization of error states. +/// The combination of colors with blinking frequency had a specific meaning. +/// +/// We'd like to drive each pair? separately, which includes: +/// - blinking (none/slow/fast) +/// - what shall blink (red/green/both-at-once/both-interlaced) +/// +/// The physical connection is not important on this level (i.e. how and what shall be sent into the shift registers) + +namespace modules { +namespace leds { + + /// Mode of LED + /// blink0 and blink1 allow for interlaced blinking of LEDs (one is on and the other off) + enum Mode { + off, + on, + blink0, ///< start blinking at even periods + blink1 ///< start blinking at odd periods + }; + + /// a single LED + class LED { + public: + constexpr inline LED() = default; + void SetMode(Mode mode); + + /// @returns true if the LED shines + bool Step(bool oddPeriod); + inline bool On() const { return state.on; } + + private: + struct State { + uint8_t on : 1; + uint8_t mode : 2; + constexpr inline State() + : on(0) + , mode(Mode::off) {} + }; + + State state; + }; + + /// main LED API + class LEDs { + public: + constexpr inline LEDs() + : ms(0) {}; + + /// step LED automaton + /// @returns statuses of LEDs - one bit per LED and 1 = on, 0 = off + uint16_t Step(uint8_t delta_ms); + + inline constexpr uint8_t LedPairsCount() const { return ledPairs; } + + inline void SetMode(uint8_t slot, bool red, Mode mode) { + SetMode(slot * 2 + red, mode); + } + inline void SetMode(uint8_t index, Mode mode) { + leds[index].SetMode(mode); + } + + inline bool LedOn(uint8_t index) const { + return leds[index].On(); + } + inline bool LedOn(uint8_t slot, bool red) const { + return leds[slot * 2 + red].On(); + } + + private: + constexpr static const uint8_t ledPairs = 5; + LED leds[ledPairs * 2]; + uint16_t ms; + }; + + //// asi nechame moznost jednu blikat rychle a druhou pomalu + + //// tohle ale nevyresi, ze jedna LED ma blikat a druha svitit + + //// problem je, ze zapnuti/vypnuti led je blink none ... mozna by to chtelo blink none-on a none-off + //// jaka bude mnozina rezimu? + //// green-off green-on green-blink-slow green-blink-fast + //// red-off + + + + + //// red-on + + + + + //// red-blink-slow + + 2 x + //// red-blink-fast + + x 2 + //// + //// rezim 2 muze mit jeste varianty - blink-sync, blink-interlaced, coz v podstate znamena rezimy: + //// blink-slow1, blink-slow2, blink-fast1, blink-fast2, pricemz nemaji smysl kombinace: + //// blink-slow + blink-fast + + //// revize + //// v aktualnim FW led jen blikaji jednou rychlosti + //// takze to spis udelam jako pole LED, pricemz cervene budou na sudych a zelene na lichych indexech + //// stav ledky bude: off, on, blink0 a blink1 + //// blink0 znamena zacni pocitat blikaci interval od sude periody + //// blink1 od liche + //// tim pujde zmodelovat jak sync tak async blikani + //// Dale je otazka, jestli chceme rychle a pomale blikani... asi ne, kdyz bude report na LCD tiskarny + + //// Dale castecne souvisejici + //// Start MMU by mel reportovat progress, pricemz aktualni stav je takovy, ze nabihaji LED a kazda neco znamena + //// Tiskarna by klidne mohla posilat Q a dostavat odpoved ve stylu "starting" a progress/state code + //// + //// Mozna taky budeme potrebovat nastavovat ruzne vnitrni promenne (treba use slots 1-3 namisto 0-4), takze operace SetVar a k tomu obracena GetVar + //// Dava mi smysl modifikovat protokol V jako getvar (zamerne ne G, aby to nekolidovalo s gcodes) a S jako setvar + //// cislo je index variable a odpovi se A_hodnota, cili accepted a hodnota odpovedi + + //void SetLEDs(uint8_t slot0, uint8_t slot1, uint8_t slot2, uint8_t slot3, uint8_t slot4); + +} // namespace LEDs +} // namespace modules diff --git a/src/modules/protocol.cpp b/src/modules/protocol.cpp index 893e039..356a30e 100644 --- a/src/modules/protocol.cpp +++ b/src/modules/protocol.cpp @@ -11,198 +11,178 @@ // F : operation finished - will be repeated to "Q" messages until a new command is issued namespace modules { +namespace protocol { -// decoding automaton -// states: input -> transition into state -// Code QTLMUXPSBEWK -> msgcode -// \n ->start -// * ->error -// error \n ->start -// * ->error -// msgcode 0-9 ->msgvalue -// * ->error -// msgvalue 0-9 ->msgvalue -// \n ->start successfully accepted command + // decoding automaton + // states: input -> transition into state + // Code QTLMUXPSBEWK -> msgcode + // \n ->start + // * ->error + // error \n ->start + // * ->error + // msgcode 0-9 ->msgvalue + // * ->error + // msgvalue 0-9 ->msgvalue + // \n ->start successfully accepted command -Protocol::DecodeStatus Protocol::DecodeRequest(uint8_t c) { - switch (rqState) { - case RequestStates::Code: - switch (c) { - case 'Q': - case 'T': - case 'L': - case 'M': - case 'U': - case 'X': - case 'P': - case 'S': - case 'B': - case 'E': - case 'W': - case 'K': - requestMsg.code = (RequestMsgCodes)c; - requestMsg.value = 0; - rqState = RequestStates::Value; - return DecodeStatus::NeedMoreData; - default: - requestMsg.code = RequestMsgCodes::unknown; - rqState = RequestStates::Error; - return DecodeStatus::Error; - } - case RequestStates::Value: - if (c >= '0' && c <= '9') { - requestMsg.value *= 10; - requestMsg.value += c - '0'; - return DecodeStatus::NeedMoreData; - } else if (c == '\n') { - rqState = RequestStates::Code; - return DecodeStatus::MessageCompleted; - } else { - requestMsg.code = RequestMsgCodes::unknown; - rqState = RequestStates::Error; - return DecodeStatus::Error; - } - default: //case error: - if (c == '\n') { - rqState = RequestStates::Code; - return DecodeStatus::MessageCompleted; - } else { - requestMsg.code = RequestMsgCodes::unknown; - rqState = RequestStates::Error; - return DecodeStatus::Error; + DecodeStatus Protocol::DecodeRequest(uint8_t c) { + switch (rqState) { + case RequestStates::Code: + switch (c) { + case 'Q': + case 'T': + case 'L': + case 'M': + case 'U': + case 'X': + case 'P': + case 'S': + case 'B': + case 'E': + case 'W': + case 'K': + requestMsg.code = (RequestMsgCodes)c; + requestMsg.value = 0; + rqState = RequestStates::Value; + return DecodeStatus::NeedMoreData; + default: + requestMsg.code = RequestMsgCodes::unknown; + rqState = RequestStates::Error; + return DecodeStatus::Error; + } + case RequestStates::Value: + if (c >= '0' && c <= '9') { + requestMsg.value *= 10; + requestMsg.value += c - '0'; + return DecodeStatus::NeedMoreData; + } else if (c == '\n') { + rqState = RequestStates::Code; + return DecodeStatus::MessageCompleted; + } else { + requestMsg.code = RequestMsgCodes::unknown; + rqState = RequestStates::Error; + return DecodeStatus::Error; + } + default: //case error: + if (c == '\n') { + rqState = RequestStates::Code; + return DecodeStatus::MessageCompleted; + } else { + requestMsg.code = RequestMsgCodes::unknown; + rqState = RequestStates::Error; + return DecodeStatus::Error; + } } } -} -uint8_t Protocol::EncodeRequest(const RequestMsg &msg, uint8_t *txbuff) { - txbuff[0] = (uint8_t)msg.code; - txbuff[1] = msg.value + '0'; - txbuff[2] = '\n'; - return 3; -} + uint8_t Protocol::EncodeRequest(const RequestMsg &msg, uint8_t *txbuff) { + txbuff[0] = (uint8_t)msg.code; + txbuff[1] = msg.value + '0'; + txbuff[2] = '\n'; + return 3; + } -Protocol::DecodeStatus Protocol::DecodeResponse(uint8_t c) { - switch (rspState) { - case ResponseStates::RequestCode: - switch (c) { - case 'Q': - case 'T': - case 'L': - case 'M': - case 'U': - case 'X': - case 'P': - case 'S': - case 'B': - case 'E': - case 'W': - case 'K': - responseMsg.request.code = (RequestMsgCodes)c; - responseMsg.request.value = 0; - rspState = ResponseStates::RequestValue; - return DecodeStatus::NeedMoreData; - default: - rspState = ResponseStates::Error; - return DecodeStatus::Error; - } - case ResponseStates::RequestValue: - if (c >= '0' && c <= '9') { - responseMsg.request.value *= 10; - responseMsg.request.value += c - '0'; - return DecodeStatus::NeedMoreData; - } else if (c == ' ') { - rspState = ResponseStates::ParamCode; - return DecodeStatus::NeedMoreData; - } else { - rspState = ResponseStates::Error; - return DecodeStatus::Error; - } - case ResponseStates::ParamCode: - switch (c) { - case 'P': - case 'E': - case 'F': - case 'A': - case 'R': - rspState = ResponseStates::ParamValue; - responseMsg.paramCode = (ResponseMsgParamCodes)c; - responseMsg.paramValue = 0; - return DecodeStatus::NeedMoreData; - default: - responseMsg.paramCode = ResponseMsgParamCodes::unknown; - rspState = ResponseStates::Error; - return DecodeStatus::Error; - } - case ResponseStates::ParamValue: - if (c >= '0' && c <= '9') { - responseMsg.paramValue *= 10; - responseMsg.paramValue += c - '0'; - return DecodeStatus::NeedMoreData; - } else if (c == '\n') { - rspState = ResponseStates::RequestCode; - return DecodeStatus::MessageCompleted; - } else { - responseMsg.paramCode = ResponseMsgParamCodes::unknown; - rspState = ResponseStates::Error; - return DecodeStatus::Error; - } - default: //case error: - if (c == '\n') { - rspState = ResponseStates::RequestCode; - return DecodeStatus::MessageCompleted; - } else { - responseMsg.paramCode = ResponseMsgParamCodes::unknown; - return DecodeStatus::Error; + DecodeStatus Protocol::DecodeResponse(uint8_t c) { + switch (rspState) { + case ResponseStates::RequestCode: + switch (c) { + case 'Q': + case 'T': + case 'L': + case 'M': + case 'U': + case 'X': + case 'P': + case 'S': + case 'B': + case 'E': + case 'W': + case 'K': + responseMsg.request.code = (RequestMsgCodes)c; + responseMsg.request.value = 0; + rspState = ResponseStates::RequestValue; + return DecodeStatus::NeedMoreData; + default: + rspState = ResponseStates::Error; + return DecodeStatus::Error; + } + case ResponseStates::RequestValue: + if (c >= '0' && c <= '9') { + responseMsg.request.value *= 10; + responseMsg.request.value += c - '0'; + return DecodeStatus::NeedMoreData; + } else if (c == ' ') { + rspState = ResponseStates::ParamCode; + return DecodeStatus::NeedMoreData; + } else { + rspState = ResponseStates::Error; + return DecodeStatus::Error; + } + case ResponseStates::ParamCode: + switch (c) { + case 'P': + case 'E': + case 'F': + case 'A': + case 'R': + rspState = ResponseStates::ParamValue; + responseMsg.paramCode = (ResponseMsgParamCodes)c; + responseMsg.paramValue = 0; + return DecodeStatus::NeedMoreData; + default: + responseMsg.paramCode = ResponseMsgParamCodes::unknown; + rspState = ResponseStates::Error; + return DecodeStatus::Error; + } + case ResponseStates::ParamValue: + if (c >= '0' && c <= '9') { + responseMsg.paramValue *= 10; + responseMsg.paramValue += c - '0'; + return DecodeStatus::NeedMoreData; + } else if (c == '\n') { + rspState = ResponseStates::RequestCode; + return DecodeStatus::MessageCompleted; + } else { + responseMsg.paramCode = ResponseMsgParamCodes::unknown; + rspState = ResponseStates::Error; + return DecodeStatus::Error; + } + default: //case error: + if (c == '\n') { + rspState = ResponseStates::RequestCode; + return DecodeStatus::MessageCompleted; + } else { + responseMsg.paramCode = ResponseMsgParamCodes::unknown; + return DecodeStatus::Error; + } } } -} -uint8_t Protocol::EncodeResponseCmdAR(const RequestMsg &msg, ResponseMsgParamCodes ar, uint8_t *txbuff) { - txbuff[0] = (uint8_t)msg.code; - txbuff[1] = msg.value + '0'; - txbuff[2] = ' '; - txbuff[3] = (uint8_t)ar; - txbuff[4] = '\n'; - return 5; -} - -uint8_t Protocol::EncodeResponseReadFINDA(const RequestMsg &msg, uint8_t findaValue, uint8_t *txbuff) { - txbuff[0] = (uint8_t)msg.code; - txbuff[1] = msg.value + '0'; - txbuff[2] = ' '; - txbuff[3] = (uint8_t)ResponseMsgParamCodes::Accepted; - txbuff[4] = findaValue + '0'; - txbuff[5] = '\n'; - return 6; -} - -uint8_t Protocol::EncodeResponseVersion(const RequestMsg &msg, uint8_t value, uint8_t *txbuff) { - txbuff[0] = (uint8_t)msg.code; - txbuff[1] = msg.value + '0'; - txbuff[2] = ' '; - txbuff[3] = (uint8_t)ResponseMsgParamCodes::Accepted; - uint8_t *dst = txbuff + 4; - if (value < 10) { - *dst++ = value + '0'; - } else if (value < 100) { - *dst++ = value / 10 + '0'; - *dst++ = value % 10 + '0'; - } else { - *dst++ = value / 100 + '0'; - *dst++ = (value / 10) % 10 + '0'; - *dst++ = value % 10 + '0'; + uint8_t Protocol::EncodeResponseCmdAR(const RequestMsg &msg, ResponseMsgParamCodes ar, uint8_t *txbuff) { + txbuff[0] = (uint8_t)msg.code; + txbuff[1] = msg.value + '0'; + txbuff[2] = ' '; + txbuff[3] = (uint8_t)ar; + txbuff[4] = '\n'; + return 5; } - *dst = '\n'; - return dst - txbuff + 1; -} -uint8_t Protocol::EncodeResponseQueryOperation(const RequestMsg &msg, ResponseMsgParamCodes code, uint8_t value, uint8_t *txbuff) { - txbuff[0] = (uint8_t)msg.code; - txbuff[1] = msg.value + '0'; - txbuff[2] = ' '; - txbuff[3] = (uint8_t)code; - uint8_t *dst = txbuff + 4; - if (code != ResponseMsgParamCodes::Finished) { + uint8_t Protocol::EncodeResponseReadFINDA(const RequestMsg &msg, uint8_t findaValue, uint8_t *txbuff) { + txbuff[0] = (uint8_t)msg.code; + txbuff[1] = msg.value + '0'; + txbuff[2] = ' '; + txbuff[3] = (uint8_t)ResponseMsgParamCodes::Accepted; + txbuff[4] = findaValue + '0'; + txbuff[5] = '\n'; + return 6; + } + + uint8_t Protocol::EncodeResponseVersion(const RequestMsg &msg, uint8_t value, uint8_t *txbuff) { + txbuff[0] = (uint8_t)msg.code; + txbuff[1] = msg.value + '0'; + txbuff[2] = ' '; + txbuff[3] = (uint8_t)ResponseMsgParamCodes::Accepted; + uint8_t *dst = txbuff + 4; if (value < 10) { *dst++ = value + '0'; } else if (value < 100) { @@ -213,9 +193,31 @@ uint8_t Protocol::EncodeResponseQueryOperation(const RequestMsg &msg, ResponseMs *dst++ = (value / 10) % 10 + '0'; *dst++ = value % 10 + '0'; } + *dst = '\n'; + return dst - txbuff + 1; } - *dst = '\n'; - return dst - txbuff + 1; -} + uint8_t Protocol::EncodeResponseQueryOperation(const RequestMsg &msg, ResponseMsgParamCodes code, uint8_t value, uint8_t *txbuff) { + txbuff[0] = (uint8_t)msg.code; + txbuff[1] = msg.value + '0'; + txbuff[2] = ' '; + txbuff[3] = (uint8_t)code; + uint8_t *dst = txbuff + 4; + if (code != ResponseMsgParamCodes::Finished) { + if (value < 10) { + *dst++ = value + '0'; + } else if (value < 100) { + *dst++ = value / 10 + '0'; + *dst++ = value % 10 + '0'; + } else { + *dst++ = value / 100 + '0'; + *dst++ = (value / 10) % 10 + '0'; + *dst++ = value % 10 + '0'; + } + } + *dst = '\n'; + return dst - txbuff + 1; + } + +} // namespace protocol } // namespace modules diff --git a/src/modules/protocol.h b/src/modules/protocol.h index 0f01a76..98a8d61 100644 --- a/src/modules/protocol.h +++ b/src/modules/protocol.h @@ -7,60 +7,56 @@ /// @@TODO possibly add some checksum to verify the correctness of messages namespace modules { +namespace protocol { -enum class RequestMsgCodes : uint8_t { - unknown = 0, - Query = 'Q', - Tool = 'T', - Load = 'L', - Mode = 'M', - Unload = 'U', - Reset = 'X', - Finda = 'P', - Version = 'S', - Button = 'B', - Eject = 'E', - Wait = 'W', - Cut = 'K' -}; + enum class RequestMsgCodes : uint8_t { + unknown = 0, + Query = 'Q', + Tool = 'T', + Load = 'L', + Mode = 'M', + Unload = 'U', + Reset = 'X', + Finda = 'P', + Version = 'S', + Button = 'B', + Eject = 'E', + Wait = 'W', + Cut = 'K' + }; -enum class ResponseMsgParamCodes : uint8_t { - unknown = 0, - Processing = 'P', - Error = 'E', - Finished = 'F', - Accepted = 'A', - Rejected = 'R' -}; + enum class ResponseMsgParamCodes : uint8_t { + unknown = 0, + Processing = 'P', + Error = 'E', + Finished = 'F', + Accepted = 'A', + Rejected = 'R' + }; -/// A request message -/// Requests are being sent by the printer into the MMU -/// It is the same structure as the generic Msg -struct RequestMsg { - RequestMsgCodes code; - uint8_t value; - inline RequestMsg(RequestMsgCodes code, uint8_t value) - : code(code) - , value(value) {} -}; + /// A request message + /// Requests are being sent by the printer into the MMU + /// It is the same structure as the generic Msg + struct RequestMsg { + RequestMsgCodes code; + uint8_t value; + inline RequestMsg(RequestMsgCodes code, uint8_t value) + : code(code) + , value(value) {} + }; -/// A response message -/// Responses are being sent from the MMU into the printer as a response to a request message -struct ResponseMsg { - RequestMsg request; ///< response is always preceeded by the request message - ResponseMsgParamCodes paramCode; ///< parameters of reply - uint8_t paramValue; ///< parameters of reply - inline ResponseMsg(RequestMsg request, ResponseMsgParamCodes paramCode, uint8_t paramValue) - : request(request) - , paramCode(paramCode) - , paramValue(paramValue) {} -}; + /// A response message + /// Responses are being sent from the MMU into the printer as a response to a request message + struct ResponseMsg { + RequestMsg request; ///< response is always preceeded by the request message + ResponseMsgParamCodes paramCode; ///< parameters of reply + uint8_t paramValue; ///< parameters of reply + inline ResponseMsg(RequestMsg request, ResponseMsgParamCodes paramCode, uint8_t paramValue) + : request(request) + , paramCode(paramCode) + , paramValue(paramValue) {} + }; -/// Protocol class is responsible for creating/decoding messages in Rx/Tx buffer -/// Beware - in the decoding more, it is meant to be a statefull instance which works through public methods -/// processing one input byte per call -class Protocol { -public: /// Message decoding return value enum class DecodeStatus : uint_fast8_t { MessageCompleted, ///< message completed and successfully lexed @@ -68,76 +64,82 @@ public: Error, ///< input character broke message decoding }; - inline Protocol() - : rqState(RequestStates::Code) - , requestMsg(RequestMsgCodes::unknown, 0) - , rspState(ResponseStates::RequestCode) - , responseMsg(RequestMsg(RequestMsgCodes::unknown, 0), ResponseMsgParamCodes::unknown, 0) { - } + /// Protocol class is responsible for creating/decoding messages in Rx/Tx buffer + /// Beware - in the decoding more, it is meant to be a statefull instance which works through public methods + /// processing one input byte per call + class Protocol { + public: + inline Protocol() + : rqState(RequestStates::Code) + , requestMsg(RequestMsgCodes::unknown, 0) + , rspState(ResponseStates::RequestCode) + , responseMsg(RequestMsg(RequestMsgCodes::unknown, 0), ResponseMsgParamCodes::unknown, 0) { + } - /// Takes the input byte c and steps one step through the state machine - /// @returns state of the message being decoded - DecodeStatus DecodeRequest(uint8_t c); + /// Takes the input byte c and steps one step through the state machine + /// @returns state of the message being decoded + DecodeStatus DecodeRequest(uint8_t c); - /// Decodes response message in rxbuff - /// @returns decoded response message structure - DecodeStatus DecodeResponse(uint8_t c); + /// Decodes response message in rxbuff + /// @returns decoded response message structure + DecodeStatus DecodeResponse(uint8_t c); - /// Encodes request message msg into txbuff memory - /// It is expected the txbuff is large enough to fit the message - /// @returns number of bytes written into txbuff - static uint8_t EncodeRequest(const RequestMsg &msg, uint8_t *txbuff); + /// Encodes request message msg into txbuff memory + /// It is expected the txbuff is large enough to fit the message + /// @returns number of bytes written into txbuff + static uint8_t EncodeRequest(const RequestMsg &msg, uint8_t *txbuff); - /// Encode generic response Command Accepted or Rejected - /// @param msg source request message for this response - /// @returns number of bytes written into txbuff - static uint8_t EncodeResponseCmdAR(const RequestMsg &msg, ResponseMsgParamCodes ar, uint8_t *txbuff); + /// Encode generic response Command Accepted or Rejected + /// @param msg source request message for this response + /// @returns number of bytes written into txbuff + static uint8_t EncodeResponseCmdAR(const RequestMsg &msg, ResponseMsgParamCodes ar, uint8_t *txbuff); - /// Encode response to Read FINDA query - /// @param msg source request message for this response - /// @param findaValue 1/0 (on/off) status of FINDA - /// @returns number of bytes written into txbuff - static uint8_t EncodeResponseReadFINDA(const RequestMsg &msg, uint8_t findaValue, uint8_t *txbuff); + /// Encode response to Read FINDA query + /// @param msg source request message for this response + /// @param findaValue 1/0 (on/off) status of FINDA + /// @returns number of bytes written into txbuff + static uint8_t EncodeResponseReadFINDA(const RequestMsg &msg, uint8_t findaValue, uint8_t *txbuff); - /// Encode response to Version query - /// @param msg source request message for this response - /// @param value version number (0-255) - /// @returns number of bytes written into txbuff - static uint8_t EncodeResponseVersion(const RequestMsg &msg, uint8_t value, uint8_t *txbuff); + /// Encode response to Version query + /// @param msg source request message for this response + /// @param value version number (0-255) + /// @returns number of bytes written into txbuff + static uint8_t EncodeResponseVersion(const RequestMsg &msg, uint8_t value, uint8_t *txbuff); - /// Encode response to Query operation status - /// @param msg source request message for this response - /// @param code status of operation (Processing, Error, Finished) - /// @param value related to status of operation(e.g. error code or progress) - /// @returns number of bytes written into txbuff - static uint8_t EncodeResponseQueryOperation(const RequestMsg &msg, ResponseMsgParamCodes code, uint8_t value, uint8_t *txbuff); + /// Encode response to Query operation status + /// @param msg source request message for this response + /// @param code status of operation (Processing, Error, Finished) + /// @param value related to status of operation(e.g. error code or progress) + /// @returns number of bytes written into txbuff + static uint8_t EncodeResponseQueryOperation(const RequestMsg &msg, ResponseMsgParamCodes code, uint8_t value, uint8_t *txbuff); - /// @returns the most recently lexed request message - inline const RequestMsg GetRequestMsg() const { return requestMsg; } + /// @returns the most recently lexed request message + inline const RequestMsg GetRequestMsg() const { return requestMsg; } - /// @returns the most recently lexed response message - inline const ResponseMsg GetResponseMsg() const { return responseMsg; } + /// @returns the most recently lexed response message + inline const ResponseMsg GetResponseMsg() const { return responseMsg; } -private: - enum class RequestStates : uint8_t { - Code, ///< starting state - expects message code - Value, ///< expecting code value - Error ///< automaton in error state + private: + enum class RequestStates : uint8_t { + Code, ///< starting state - expects message code + Value, ///< expecting code value + Error ///< automaton in error state + }; + + RequestStates rqState; + RequestMsg requestMsg; + + enum class ResponseStates : uint8_t { + RequestCode, ///< starting state - expects message code + RequestValue, ///< expecting code value + ParamCode, ///< expecting param code + ParamValue, ///< expecting param value + Error ///< automaton in error state + }; + + ResponseStates rspState; + ResponseMsg responseMsg; }; - RequestStates rqState; - RequestMsg requestMsg; - - enum class ResponseStates : uint8_t { - RequestCode, ///< starting state - expects message code - RequestValue, ///< expecting code value - ParamCode, ///< expecting param code - ParamValue, ///< expecting param value - Error ///< automaton in error state - }; - - ResponseStates rspState; - ResponseMsg responseMsg; -}; - +} // namespace protocol } // namespace modules diff --git a/tests/unit/modules/buttons/stub_adc.cpp b/tests/unit/modules/buttons/stub_adc.cpp index c6edfa3..5006f68 100644 --- a/tests/unit/modules/buttons/stub_adc.cpp +++ b/tests/unit/modules/buttons/stub_adc.cpp @@ -3,7 +3,7 @@ #include namespace hal { -namespace ADC { +namespace adc { static TADCData values2Return; static TADCData::const_iterator rdptr = values2Return.cbegin(); @@ -28,5 +28,5 @@ namespace ADC { return rdptr != values2Return.end() ? *rdptr : 1023; } -} // namespace ADC +} // namespace adc } // namespace hal diff --git a/tests/unit/modules/buttons/stub_adc.h b/tests/unit/modules/buttons/stub_adc.h index 7105f01..ae3b658 100644 --- a/tests/unit/modules/buttons/stub_adc.h +++ b/tests/unit/modules/buttons/stub_adc.h @@ -4,11 +4,11 @@ #include namespace hal { -namespace ADC { +namespace adc { using TADCData = std::vector; void ReinitADC(TADCData &&d, uint8_t ovsmpl); -} // namespace ADC +} // namespace adc } // namespace hal diff --git a/tests/unit/modules/buttons/test_buttons.cpp b/tests/unit/modules/buttons/test_buttons.cpp index ded5db0..261edf9 100644 --- a/tests/unit/modules/buttons/test_buttons.cpp +++ b/tests/unit/modules/buttons/test_buttons.cpp @@ -4,21 +4,21 @@ using Catch::Matchers::Equals; -bool Step_Basic_One_Button_Test(modules::Buttons &b, uint8_t oversampleFactor, uint8_t testedButtonIndex, uint8_t otherButton1, uint8_t otherButton2) { +bool Step_Basic_One_Button_Test(modules::buttons::Buttons &b, uint8_t oversampleFactor, uint8_t testedButtonIndex, uint8_t otherButton1, uint8_t otherButton2) { for (uint8_t i = 0; i < oversampleFactor; ++i) - b.Step(); // should detect the press but remain in detected state - wait for debounce + b.Step(hal::adc::ReadADC(0)); // should detect the press but remain in detected state - wait for debounce CHECK(!b.ButtonPressed(testedButtonIndex)); CHECK(!b.ButtonPressed(otherButton1)); CHECK(!b.ButtonPressed(otherButton2)); for (uint8_t i = 0; i < oversampleFactor; ++i) - b.Step(); // reset to waiting + b.Step(hal::adc::ReadADC(0)); // reset to waiting CHECK(b.ButtonPressed(testedButtonIndex)); CHECK(!b.ButtonPressed(otherButton1)); CHECK(!b.ButtonPressed(otherButton2)); for (uint8_t i = 0; i < oversampleFactor; ++i) - b.Step(); // pressed again, still in debouncing state + b.Step(hal::adc::ReadADC(0)); // pressed again, still in debouncing state CHECK(!b.ButtonPressed(testedButtonIndex)); CHECK(!b.ButtonPressed(otherButton1)); CHECK(!b.ButtonPressed(otherButton2)); @@ -27,14 +27,14 @@ bool Step_Basic_One_Button_Test(modules::Buttons &b, uint8_t oversampleFactor, u } /// This test verifies the behaviour of a single button. The other buttons must remain intact. -bool Step_Basic_One_Button(hal::ADC::TADCData &&d, uint8_t testedButtonIndex) { - using namespace modules; +bool Step_Basic_One_Button(hal::adc::TADCData &&d, uint8_t testedButtonIndex) { + using namespace modules::buttons; Buttons b; // need to oversample the data as debouncing takes 100 cycles to accept a pressed button constexpr uint8_t oversampleFactor = 100; - hal::ADC::ReinitADC(std::move(d), oversampleFactor); + hal::adc::ReinitADC(std::move(d), oversampleFactor); uint8_t otherButton1 = 1, otherButton2 = 2; switch (testedButtonIndex) { @@ -53,15 +53,15 @@ bool Step_Basic_One_Button(hal::ADC::TADCData &&d, uint8_t testedButtonIndex) { TEST_CASE("buttons::Step-basic-button", "[buttons]") { { - hal::ADC::TADCData d({ 5, 6, 1023 }); + hal::adc::TADCData d({ 5, 6, 1023 }); CHECK(Step_Basic_One_Button(std::move(d), 0)); } { - hal::ADC::TADCData d({ 321, 359, 1023 }); + hal::adc::TADCData d({ 321, 359, 1023 }); CHECK(Step_Basic_One_Button(std::move(d), 1)); } { - hal::ADC::TADCData d({ 501, 529, 1023 }); + hal::adc::TADCData d({ 501, 529, 1023 }); CHECK(Step_Basic_One_Button(std::move(d), 2)); } } @@ -70,13 +70,13 @@ TEST_CASE("buttons::Step-basic-button", "[buttons]") { /// and the Buttons class should press first button and release, then the second one and then the third one /// without being reinitialized. TEST_CASE("buttons::Step-basic-button-one-after-other", "[buttons]") { - using namespace modules; - hal::ADC::TADCData d({ 5, 6, 1023, 321, 359, 1023, 501, 529, 1023 }); + using namespace modules::buttons; + hal::adc::TADCData d({ 5, 6, 1023, 321, 359, 1023, 501, 529, 1023 }); Buttons b; // need to oversample the data as debouncing takes 100 cycles to accept a pressed button constexpr uint8_t oversampleFactor = 100; - hal::ADC::ReinitADC(std::move(d), oversampleFactor); + hal::adc::ReinitADC(std::move(d), oversampleFactor); CHECK(Step_Basic_One_Button_Test(b, oversampleFactor, 0, 1, 2)); CHECK(Step_Basic_One_Button_Test(b, oversampleFactor, 1, 0, 2)); @@ -85,76 +85,76 @@ TEST_CASE("buttons::Step-basic-button-one-after-other", "[buttons]") { /// This test tries to simulate a bouncing effect on data from ADC on the first button TEST_CASE("buttons::Step-debounce-one-button", "[buttons]") { - using namespace modules; + using namespace modules::buttons; // make a bounce event on the first press - hal::ADC::TADCData d({ 5, 1023, 5, 9, 6, 7, 8, 1023, 1023 }); + hal::adc::TADCData d({ 5, 1023, 5, 9, 6, 7, 8, 1023, 1023 }); // need to oversample the data as debouncing takes 100 cycles to accept a pressed button constexpr uint8_t oversampleFactor = 25; - hal::ADC::ReinitADC(std::move(d), oversampleFactor); + hal::adc::ReinitADC(std::move(d), oversampleFactor); Buttons b; // 5 for (uint8_t i = 0; i < oversampleFactor; ++i) - b.Step(); // should detect the press but remain in detected state - wait for debounce + b.Step(hal::adc::ReadADC(0)); // should detect the press but remain in detected state - wait for debounce CHECK(!b.ButtonPressed(0)); CHECK(!b.ButtonPressed(1)); CHECK(!b.ButtonPressed(2)); // 1023 for (uint8_t i = 0; i < oversampleFactor; ++i) - b.Step(); // reset to waiting + b.Step(hal::adc::ReadADC(0)); // reset to waiting CHECK(!b.ButtonPressed(0)); CHECK(!b.ButtonPressed(1)); CHECK(!b.ButtonPressed(2)); // 5 for (uint8_t i = 0; i < oversampleFactor; ++i) - b.Step(); // pressed again, still in debouncing state + b.Step(hal::adc::ReadADC(0)); // pressed again, still in debouncing state CHECK(!b.ButtonPressed(0)); CHECK(!b.ButtonPressed(1)); CHECK(!b.ButtonPressed(2)); // 9 for (uint8_t i = 0; i < oversampleFactor; ++i) - b.Step(); // no change + b.Step(hal::adc::ReadADC(0)); // no change CHECK(!b.ButtonPressed(0)); CHECK(!b.ButtonPressed(1)); CHECK(!b.ButtonPressed(2)); // 6 for (uint8_t i = 0; i < oversampleFactor; ++i) - b.Step(); // no change + b.Step(hal::adc::ReadADC(0)); // no change CHECK(!b.ButtonPressed(0)); CHECK(!b.ButtonPressed(1)); CHECK(!b.ButtonPressed(2)); // 7 for (uint8_t i = 0; i < oversampleFactor; ++i) - b.Step(); // one step from "pressed" + b.Step(hal::adc::ReadADC(0)); // one step from "pressed" CHECK(!b.ButtonPressed(0)); CHECK(!b.ButtonPressed(1)); CHECK(!b.ButtonPressed(2)); // 8 for (uint8_t i = 0; i < oversampleFactor; ++i) - b.Step(); // fifth set of samples - should report "pressed" finally + b.Step(hal::adc::ReadADC(0)); // fifth set of samples - should report "pressed" finally CHECK(b.ButtonPressed(0)); CHECK(!b.ButtonPressed(1)); CHECK(!b.ButtonPressed(2)); // 1023 for (uint8_t i = 0; i < oversampleFactor; ++i) - b.Step(); // sixth set of samples - button released (no debouncing on release) + b.Step(hal::adc::ReadADC(0)); // sixth set of samples - button released (no debouncing on release) CHECK(!b.ButtonPressed(0)); CHECK(!b.ButtonPressed(1)); CHECK(!b.ButtonPressed(2)); // 1023 for (uint8_t i = 0; i < oversampleFactor; ++i) - b.Step(); // seventh set of samples - still released + b.Step(hal::adc::ReadADC(0)); // seventh set of samples - still released CHECK(!b.ButtonPressed(0)); CHECK(!b.ButtonPressed(1)); CHECK(!b.ButtonPressed(2)); diff --git a/tests/unit/modules/protocol/test_protocol.cpp b/tests/unit/modules/protocol/test_protocol.cpp index a134428..a2e461d 100644 --- a/tests/unit/modules/protocol/test_protocol.cpp +++ b/tests/unit/modules/protocol/test_protocol.cpp @@ -4,7 +4,7 @@ using Catch::Matchers::Equals; TEST_CASE("protocol::EncodeRequests", "[protocol]") { - using namespace modules; + using namespace modules::protocol; RequestMsgCodes code; uint8_t value; @@ -45,7 +45,7 @@ TEST_CASE("protocol::EncodeRequests", "[protocol]") { } TEST_CASE("protocol::EncodeResponseCmdAR", "[protocol]") { - using namespace modules; + using namespace modules::protocol; auto requestMsg = GENERATE( RequestMsg(RequestMsgCodes::Button, 0), @@ -93,7 +93,7 @@ TEST_CASE("protocol::EncodeResponseCmdAR", "[protocol]") { } TEST_CASE("protocol::EncodeResponseReadFINDA", "[protocol]") { - using namespace modules; + using namespace modules::protocol; auto requestMsg = RequestMsg(RequestMsgCodes::Finda, 0); uint8_t findaStatus = GENERATE(0, 1); @@ -111,7 +111,7 @@ TEST_CASE("protocol::EncodeResponseReadFINDA", "[protocol]") { } TEST_CASE("protocol::EncodeResponseVersion", "[protocol]") { - using namespace modules; + using namespace modules::protocol; std::uint8_t versionQueryType = GENERATE(0, 1, 2, 3); auto requestMsg = RequestMsg(RequestMsgCodes::Version, versionQueryType); @@ -142,7 +142,7 @@ TEST_CASE("protocol::EncodeResponseVersion", "[protocol]") { } TEST_CASE("protocol::EncodeResponseQueryOperation", "[protocol]") { - using namespace modules; + using namespace modules::protocol; auto requestMsg = GENERATE( RequestMsg(RequestMsgCodes::Cut, 0), @@ -202,7 +202,7 @@ TEST_CASE("protocol::EncodeResponseQueryOperation", "[protocol]") { } TEST_CASE("protocol::DecodeRequest", "[protocol]") { - using namespace modules; + using namespace modules::protocol; Protocol p; const char *rxbuff = GENERATE( "B0\n", "B1\n", "B2\n", @@ -226,9 +226,9 @@ TEST_CASE("protocol::DecodeRequest", "[protocol]") { break; } else if (c == '\n') { // regular end of message line - CHECK(p.DecodeRequest(c) == Protocol::DecodeStatus::MessageCompleted); + CHECK(p.DecodeRequest(c) == DecodeStatus::MessageCompleted); } else { - CHECK(p.DecodeRequest(c) == Protocol::DecodeStatus::NeedMoreData); + CHECK(p.DecodeRequest(c) == DecodeStatus::NeedMoreData); } } @@ -239,7 +239,7 @@ TEST_CASE("protocol::DecodeRequest", "[protocol]") { } TEST_CASE("protocol::DecodeResponseReadFinda", "[protocol]") { - using namespace modules; + using namespace modules::protocol; Protocol p; const char *rxbuff = GENERATE( "P0 A0\n", @@ -253,9 +253,9 @@ TEST_CASE("protocol::DecodeResponseReadFinda", "[protocol]") { break; } else if (c == '\n') { // regular end of message line - CHECK(p.DecodeResponse(c) == Protocol::DecodeStatus::MessageCompleted); + CHECK(p.DecodeResponse(c) == DecodeStatus::MessageCompleted); } else { - CHECK(p.DecodeResponse(c) == Protocol::DecodeStatus::NeedMoreData); + CHECK(p.DecodeResponse(c) == DecodeStatus::NeedMoreData); } } @@ -268,7 +268,7 @@ TEST_CASE("protocol::DecodeResponseReadFinda", "[protocol]") { } TEST_CASE("protocol::DecodeResponseQueryOperation", "[protocol]") { - using namespace modules; + using namespace modules::protocol; Protocol p; const char *cmdReference = GENERATE( "E0", "E1", "E2", "E3", "E4", @@ -294,9 +294,9 @@ TEST_CASE("protocol::DecodeResponseQueryOperation", "[protocol]") { break; } else if (c == '\n') { // regular end of message line - CHECK(p.DecodeResponse(c) == Protocol::DecodeStatus::MessageCompleted); + CHECK(p.DecodeResponse(c) == DecodeStatus::MessageCompleted); } else { - CHECK(p.DecodeResponse(c) == Protocol::DecodeStatus::NeedMoreData); + CHECK(p.DecodeResponse(c) == DecodeStatus::NeedMoreData); } } @@ -311,81 +311,81 @@ TEST_CASE("protocol::DecodeResponseQueryOperation", "[protocol]") { } TEST_CASE("protocol::DecodeRequestErrors", "[protocol]") { - using namespace modules; + using namespace modules::protocol; Protocol p; const char b0[] = "b0"; - CHECK(p.DecodeRequest(b0[0]) == Protocol::DecodeStatus::Error); + CHECK(p.DecodeRequest(b0[0]) == DecodeStatus::Error); CHECK(p.GetRequestMsg().code == RequestMsgCodes::unknown); - CHECK(p.DecodeRequest(b0[1]) == Protocol::DecodeStatus::Error); + CHECK(p.DecodeRequest(b0[1]) == DecodeStatus::Error); CHECK(p.GetRequestMsg().code == RequestMsgCodes::unknown); // reset protokol decoder - CHECK(p.DecodeRequest('\n') == Protocol::DecodeStatus::MessageCompleted); + CHECK(p.DecodeRequest('\n') == DecodeStatus::MessageCompleted); CHECK(p.GetRequestMsg().code == RequestMsgCodes::unknown); const char B1_[] = "B1 \n"; - CHECK(p.DecodeRequest(B1_[0]) == Protocol::DecodeStatus::NeedMoreData); - CHECK(p.DecodeRequest(B1_[1]) == Protocol::DecodeStatus::NeedMoreData); - CHECK(p.DecodeRequest(B1_[2]) == Protocol::DecodeStatus::Error); + CHECK(p.DecodeRequest(B1_[0]) == DecodeStatus::NeedMoreData); + CHECK(p.DecodeRequest(B1_[1]) == DecodeStatus::NeedMoreData); + CHECK(p.DecodeRequest(B1_[2]) == DecodeStatus::Error); CHECK(p.GetRequestMsg().code == RequestMsgCodes::unknown); - CHECK(p.DecodeRequest(B1_[3]) == Protocol::DecodeStatus::MessageCompleted); + CHECK(p.DecodeRequest(B1_[3]) == DecodeStatus::MessageCompleted); CHECK(p.GetRequestMsg().code == RequestMsgCodes::unknown); const char _B2[] = " B2\n"; - CHECK(p.DecodeRequest(_B2[0]) == Protocol::DecodeStatus::Error); + CHECK(p.DecodeRequest(_B2[0]) == DecodeStatus::Error); CHECK(p.GetRequestMsg().code == RequestMsgCodes::unknown); - CHECK(p.DecodeRequest(_B2[1]) == Protocol::DecodeStatus::Error); + CHECK(p.DecodeRequest(_B2[1]) == DecodeStatus::Error); CHECK(p.GetRequestMsg().code == RequestMsgCodes::unknown); CHECK(p.GetRequestMsg().code == RequestMsgCodes::unknown); - CHECK(p.DecodeRequest(_B2[2]) == Protocol::DecodeStatus::Error); + CHECK(p.DecodeRequest(_B2[2]) == DecodeStatus::Error); CHECK(p.GetRequestMsg().code == RequestMsgCodes::unknown); - CHECK(p.DecodeRequest(_B2[3]) == Protocol::DecodeStatus::MessageCompleted); + CHECK(p.DecodeRequest(_B2[3]) == DecodeStatus::MessageCompleted); CHECK(p.GetRequestMsg().code == RequestMsgCodes::unknown); const char _B0_[] = " B0 "; - CHECK(p.DecodeRequest(_B0_[0]) == Protocol::DecodeStatus::Error); + CHECK(p.DecodeRequest(_B0_[0]) == DecodeStatus::Error); CHECK(p.GetRequestMsg().code == RequestMsgCodes::unknown); - CHECK(p.DecodeRequest(_B0_[1]) == Protocol::DecodeStatus::Error); + CHECK(p.DecodeRequest(_B0_[1]) == DecodeStatus::Error); CHECK(p.GetRequestMsg().code == RequestMsgCodes::unknown); - CHECK(p.DecodeRequest(_B0_[2]) == Protocol::DecodeStatus::Error); + CHECK(p.DecodeRequest(_B0_[2]) == DecodeStatus::Error); CHECK(p.GetRequestMsg().code == RequestMsgCodes::unknown); - CHECK(p.DecodeRequest(_B0_[3]) == Protocol::DecodeStatus::Error); + CHECK(p.DecodeRequest(_B0_[3]) == DecodeStatus::Error); CHECK(p.GetRequestMsg().code == RequestMsgCodes::unknown); - CHECK(p.DecodeRequest('\n') == Protocol::DecodeStatus::MessageCompleted); + CHECK(p.DecodeRequest('\n') == DecodeStatus::MessageCompleted); CHECK(p.GetRequestMsg().code == RequestMsgCodes::unknown); } TEST_CASE("protocol::DecodeResponseErrors", "[protocol]") { - using namespace modules; + using namespace modules::protocol; Protocol p; const char b0[] = "b0 A\n"; - CHECK(p.DecodeRequest(b0[0]) == Protocol::DecodeStatus::Error); + CHECK(p.DecodeRequest(b0[0]) == DecodeStatus::Error); CHECK(p.GetRequestMsg().code == RequestMsgCodes::unknown); - CHECK(p.DecodeRequest(b0[1]) == Protocol::DecodeStatus::Error); + CHECK(p.DecodeRequest(b0[1]) == DecodeStatus::Error); CHECK(p.GetRequestMsg().code == RequestMsgCodes::unknown); - CHECK(p.DecodeRequest(b0[2]) == Protocol::DecodeStatus::Error); + CHECK(p.DecodeRequest(b0[2]) == DecodeStatus::Error); CHECK(p.GetRequestMsg().code == RequestMsgCodes::unknown); - CHECK(p.DecodeRequest(b0[3]) == Protocol::DecodeStatus::Error); + CHECK(p.DecodeRequest(b0[3]) == DecodeStatus::Error); CHECK(p.GetRequestMsg().code == RequestMsgCodes::unknown); - CHECK(p.DecodeRequest(b0[4]) == Protocol::DecodeStatus::MessageCompleted); + CHECK(p.DecodeRequest(b0[4]) == DecodeStatus::MessageCompleted); CHECK(p.GetRequestMsg().code == RequestMsgCodes::unknown); const char b1[] = "b0A\n"; - CHECK(p.DecodeRequest(b1[0]) == Protocol::DecodeStatus::Error); + CHECK(p.DecodeRequest(b1[0]) == DecodeStatus::Error); CHECK(p.GetRequestMsg().code == RequestMsgCodes::unknown); - CHECK(p.DecodeRequest(b1[1]) == Protocol::DecodeStatus::Error); + CHECK(p.DecodeRequest(b1[1]) == DecodeStatus::Error); CHECK(p.GetRequestMsg().code == RequestMsgCodes::unknown); - CHECK(p.DecodeRequest(b1[2]) == Protocol::DecodeStatus::Error); + CHECK(p.DecodeRequest(b1[2]) == DecodeStatus::Error); CHECK(p.GetRequestMsg().code == RequestMsgCodes::unknown); - CHECK(p.DecodeRequest(b1[3]) == Protocol::DecodeStatus::MessageCompleted); + CHECK(p.DecodeRequest(b1[3]) == DecodeStatus::MessageCompleted); CHECK(p.GetRequestMsg().code == RequestMsgCodes::unknown); } // Beware - this test makes 18M+ combinations, run only when changing the implementation of the codec // Therefore it is disabled [.] by default TEST_CASE("protocol::DecodeResponseErrorsCross", "[protocol][.]") { - using namespace modules; + using namespace modules::protocol; Protocol p; const char *validInitialSpaces = ""; @@ -434,7 +434,7 @@ TEST_CASE("protocol::DecodeResponseErrorsCross", "[protocol][.]") { bool shouldPass = viInitialSpace && viReqCode && /*viReqValue && */ viSpace && viRspCode && viTerminatingSpaces; bool failed = false; std::for_each(msg.cbegin(), msg.cend(), [&](uint8_t c) { - if (p.DecodeResponse(c) == Protocol::DecodeStatus::Error) { + if (p.DecodeResponse(c) == DecodeStatus::Error) { failed = true; } }); From 6cb072ce796e8582427a3d2db8cdc04e65a0901a Mon Sep 17 00:00:00 2001 From: "D.R.racer" Date: Tue, 25 May 2021 11:55:28 +0200 Subject: [PATCH 2/7] Rebase onto main + clean up the code a bit --- CMakeLists.txt | 10 ++- src/hal/adc.cpp | 9 +++ src/hal/avr/usart.cpp | 124 +++++++++++++++++--------------- src/hal/usart.h | 159 ++++++++++++++++++++++-------------------- src/main.cpp | 36 +++++----- 5 files changed, 184 insertions(+), 154 deletions(-) create mode 100644 src/hal/adc.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 239ae85..16f8f69 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -183,8 +183,14 @@ target_include_directories(firmware PRIVATE include src) target_compile_options(firmware PRIVATE -Wdouble-promotion) target_sources( - firmware PRIVATE src/main.cpp src/hal/avr/cpu.cpp src/hal/avr/usart.cpp src/modules/protocol.cpp - src/modules/buttons.cpp src/modules/leds.cpp + firmware + PRIVATE src/main.cpp + src/hal/avr/cpu.cpp + src/hal/avr/usart.cpp + src/hal/adc.cpp + src/modules/protocol.cpp + src/modules/buttons.cpp + src/modules/leds.cpp ) set_property( diff --git a/src/hal/adc.cpp b/src/hal/adc.cpp new file mode 100644 index 0000000..c97b9c3 --- /dev/null +++ b/src/hal/adc.cpp @@ -0,0 +1,9 @@ +#include "adc.h" + +namespace hal { +namespace adc { + + uint16_t ReadADC(uint8_t adc) { return 0; } + +} // namespace adc +} // namespace hal diff --git a/src/hal/avr/usart.cpp b/src/hal/avr/usart.cpp index dc64b3e..1dac360 100644 --- a/src/hal/avr/usart.cpp +++ b/src/hal/avr/usart.cpp @@ -1,72 +1,82 @@ #include "../usart.h" #include -uint8_t hal::USART::Read() { - uint8_t c = 0; - this->rx_buf.ConsumeFirst(c); - return c; -} +namespace hal { +namespace usart { -void hal::USART::Write(uint8_t c) { - _written = true; - // If the buffer and the data register is empty, just write the byte - // to the data register and be done. This shortcut helps - // significantly improve the effective datarate at high (> - // 500kbit/s) bitrates, where interrupt overhead becomes a slowdown. - if (tx_buf.IsEmpty() && (husart->UCSRxA & (1 << 5))) { - husart->UDRx = c; - husart->UCSRxA |= (1 << 6); - return; + USART usart1; + + uint8_t USART::Read() { + uint8_t c = 0; + rx_buf.ConsumeFirst(c); + return c; } - // If the output buffer is full, there's nothing for it other than to - // wait for the interrupt handler to empty it a bit - while (!tx_buf.push_back_DontRewrite(c)) { - if (bit_is_clear(SREG, SREG_I)) { - // Interrupts are disabled, so we'll have to poll the data - // register empty flag ourselves. If it is set, pretend an - // interrupt has happened and call the handler to free up - // space for us. - if (husart->UCSRxA & (1 << 5)) - ISR_UDRE(); - } else { - // nop, the interrupt handler will free up space for us + void USART::Write(uint8_t c) { + _written = true; + // If the buffer and the data register is empty, just write the byte + // to the data register and be done. This shortcut helps + // significantly improve the effective datarate at high (> + // 500kbit/s) bitrates, where interrupt overhead becomes a slowdown. + if (tx_buf.IsEmpty() && (husart->UCSRxA & (1 << 5))) { + husart->UDRx = c; + husart->UCSRxA |= (1 << 6); + return; + } + + // If the output buffer is full, there's nothing for it other than to + // wait for the interrupt handler to empty it a bit + while (!tx_buf.push_back_DontRewrite(c)) { + if (bit_is_clear(SREG, SREG_I)) { + // Interrupts are disabled, so we'll have to poll the data + // register empty flag ourselves. If it is set, pretend an + // interrupt has happened and call the handler to free up + // space for us. + if (husart->UCSRxA & (1 << 5)) { + ISR_UDRE(); + } + } else { + // nop, the interrupt handler will free up space for us + } + } + + husart->UCSRxB |= (1 << 5); //enable UDRE interrupt + } + + void USART::Flush() { + // If we have never written a byte, no need to flush. This special + // case is needed since there is no way to force the TXC (transmit + // complete) bit to 1 during initialization + if (!_written) { + return; + } + + while ((husart->UCSRxB & (1 << 5)) || ~(husart->UCSRxA & (1 << 6))) { + if (bit_is_clear(SREG, SREG_I) && (husart->UCSRxB & (1 << 5))) + // Interrupts are globally disabled, but the DR empty + // interrupt should be enabled, so poll the DR empty flag to + // prevent deadlock + if (husart->UCSRxA & (1 << 5)) { + ISR_UDRE(); + } + } + // If we get here, nothing is queued anymore (DRIE is disabled) and + // the hardware finished tranmission (TXC is set). + } + + void USART::puts(const char *str) { + while (*str) { + Write(*str++); } } - husart->UCSRxB |= (1 << 5); //enable UDRE interrupt -} - -void hal::USART::Flush() { - // If we have never written a byte, no need to flush. This special - // case is needed since there is no way to force the TXC (transmit - // complete) bit to 1 during initialization - if (!_written) - return; - - while ((husart->UCSRxB & (1 << 5)) || ~(husart->UCSRxA & (1 << 6))) { - if (bit_is_clear(SREG, SREG_I) && (husart->UCSRxB & (1 << 5))) - // Interrupts are globally disabled, but the DR empty - // interrupt should be enabled, so poll the DR empty flag to - // prevent deadlock - if (husart->UCSRxA & (1 << 5)) - ISR_UDRE(); - } - // If we get here, nothing is queued anymore (DRIE is disabled) and - // the hardware finished tranmission (TXC is set). -} - -void hal::USART::puts(const char *str) { - while (*str) - this->Write(*str++); -} - -hal::USART usart1(USART1); +} // namespace usart +} // namespace hal ISR(USART1_RX_vect) { - usart1.ISR_RX(); + hal::usart::usart1.ISR_RX(); } ISR(USART1_UDRE_vect) { - usart1.ISR_UDRE(); + hal::usart::usart1.ISR_UDRE(); } diff --git a/src/hal/usart.h b/src/hal/usart.h index 06b91ce..080e5cc 100644 --- a/src/hal/usart.h +++ b/src/hal/usart.h @@ -8,92 +8,99 @@ /// for >1 USART interfaces namespace hal { -class USART { -public: - struct USART_TypeDef { - volatile uint8_t UCSRxA; - volatile uint8_t UCSRxB; - volatile uint8_t UCSRxC; - volatile uint8_t UCSRxD; - volatile uint16_t UBRRx; - volatile uint8_t UDRx; - }; +namespace usart { - struct USART_InitTypeDef { - hal::gpio::GPIO_pin rx_pin; - hal::gpio::GPIO_pin tx_pin; - uint32_t baudrate; - }; + class USART { + public: + struct USART_TypeDef { + volatile uint8_t UCSRxA; + volatile uint8_t UCSRxB; + volatile uint8_t UCSRxC; + volatile uint8_t UCSRxD; + volatile uint16_t UBRRx; + volatile uint8_t UDRx; + }; - /// @returns current character from the UART without extracting it from the read buffer - uint8_t Peek() const { - return rx_buf.GetFirstIfAble(); - } - /// @returns true if there are no bytes to be read - bool ReadEmpty() const { - return rx_buf.IsEmpty(); - } - /// @returns current character from the UART and extracts it from the read buffer - uint8_t Read(); + struct USART_InitTypeDef { + hal::gpio::GPIO_pin rx_pin; + hal::gpio::GPIO_pin tx_pin; + uint32_t baudrate; + }; - /// @param c character to be pushed into the TX buffer (to be sent) - void Write(uint8_t c); - /// @param str c string to be sent. NL is appended - void puts(const char *str); - /// @returns true if there is at least one byte free in the TX buffer (i.e. some space to add a character to be sent) - bool CanWrite() const { - return tx_buf.CanPush(); - } - /// blocks until the TX buffer was successfully transmitted - void Flush(); - - /// Initializes USART interface - __attribute__((always_inline)) inline void Init(USART_InitTypeDef *const conf) { - gpio::Init(conf->rx_pin, gpio::GPIO_InitTypeDef(gpio::Mode::input, gpio::Level::low)); - gpio::Init(conf->tx_pin, gpio::GPIO_InitTypeDef(gpio::Mode::output, gpio::Level::low)); - husart->UBRRx = (((double)(F_CPU)) / (((double)(conf->baudrate)) * 8.0) - 1.0 + 0.5); - husart->UCSRxA = (1 << 1); // Set double baudrate setting. Clear all other status bits/flags - // husart->UCSRxC |= (1 << 3); // 2 stop bits. Preserve data size setting - husart->UCSRxD = 0; //disable hardware flow control. This register is reserved on all AVR devides with USART. - husart->UCSRxB = (1 << 3) | (1 << 4) | (1 << 7); // Turn on the transmission and reception circuitry and enable the RX interrupt - } - - /// implementation of the receive ISR's body - __attribute__((always_inline)) inline void ISR_RX() { - if (husart->UCSRxA & (1 << 4)) { - (void)husart->UDRx; - } else { - rx_buf.push_back_DontRewrite(husart->UDRx); + /// @returns current character from the UART without extracting it from the read buffer + uint8_t Peek() const { + return rx_buf.GetFirstIfAble(); } - } - /// implementation of the transmit ISR's body - __attribute__((always_inline)) inline void ISR_UDRE() { - uint8_t c = 0; - tx_buf.ConsumeFirst(c); - husart->UDRx = c; + /// @returns true if there are no bytes to be read + bool ReadEmpty() const { + return rx_buf.IsEmpty(); + } + /// @returns current character from the UART and extracts it from the read buffer + uint8_t Read(); - // clear the TXC bit -- "can be cleared by writing a one to its bit - // location". This makes sure flush() won't return until the bytes - // actually got written - husart->UCSRxA |= (1 << 6); + /// @param c character to be pushed into the TX buffer (to be sent) + void Write(uint8_t c); + /// @param str c string to be sent. NL is appended + void puts(const char *str); + /// @returns true if there is at least one byte free in the TX buffer (i.e. some space to add a character to be sent) + bool CanWrite() const { + return tx_buf.CanPush(); + } + /// blocks until the TX buffer was successfully transmitted + void Flush(); - if (tx_buf.IsEmpty()) - husart->UCSRxB &= ~(1 << 5); // disable UDRE interrupt - } + /// Initializes USART interface + __attribute__((always_inline)) inline void Init(USART_InitTypeDef *const conf) { + gpio::Init(conf->rx_pin, gpio::GPIO_InitTypeDef(gpio::Mode::input, gpio::Level::low)); + gpio::Init(conf->tx_pin, gpio::GPIO_InitTypeDef(gpio::Mode::output, gpio::Level::low)); + husart->UBRRx = (((double)(F_CPU)) / (((double)(conf->baudrate)) * 8.0) - 1.0 + 0.5); + husart->UCSRxA = (1 << 1); // Set double baudrate setting. Clear all other status bits/flags + // husart->UCSRxC |= (1 << 3); // 2 stop bits. Preserve data size setting + husart->UCSRxD = 0; //disable hardware flow control. This register is reserved on all AVR devides with USART. + husart->UCSRxB = (1 << 3) | (1 << 4) | (1 << 7); // Turn on the transmission and reception circuitry and enable the RX interrupt + } - USART(USART_TypeDef *husart) - : husart(husart) {}; + /// implementation of the receive ISR's body + __attribute__((always_inline)) inline void ISR_RX() { + if (husart->UCSRxA & (1 << 4)) { + (void)husart->UDRx; + } else { + rx_buf.push_back_DontRewrite(husart->UDRx); + } + } + /// implementation of the transmit ISR's body + __attribute__((always_inline)) inline void ISR_UDRE() { + uint8_t c = 0; + tx_buf.ConsumeFirst(c); + husart->UDRx = c; -private: - // IO base address - USART_TypeDef *husart; - bool _written; + // clear the TXC bit -- "can be cleared by writing a one to its bit + // location". This makes sure flush() won't return until the bytes + // actually got written + husart->UCSRxA |= (1 << 6); - CircleBuffer tx_buf; - CircleBuffer rx_buf; -}; + if (tx_buf.IsEmpty()) + husart->UCSRxB &= ~(1 << 5); // disable UDRE interrupt + } + USART() = default; + void Init(USART_TypeDef *conf) { + husart = conf; + } + + private: + // IO base address + USART_TypeDef *husart; + bool _written; + + CircleBuffer tx_buf; + CircleBuffer rx_buf; + }; + + /// beware - normally we'd make a singleton, but avr-gcc generates suboptimal code for them, therefore we only keep this extern variable + extern USART usart1; + +} // namespace usart } // namespace hal #define USART1 ((hal::USART::USART_TypeDef *)&UCSR1A) -extern hal::USART usart1; diff --git a/src/main.cpp b/src/main.cpp index cb72bca..0667feb 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -13,8 +13,6 @@ #include "logic/mm_control.h" -static hal::UART uart; - static modules::protocol::Protocol protocol; static modules::buttons::Buttons buttons; static modules::leds::LEDs leds; @@ -48,18 +46,18 @@ void TmpPlayground() { // if (hal::gpio::ReadPin(GPIO_pin(GPIOB, 7)) == hal::gpio::Level::low) // break; - sei(); - usart1.puts("1234567890\n"); - usart1.puts("1234567890\n"); - usart1.puts("1234567890\n"); - usart1.puts("1234567890\n"); - usart1.puts("1234567890\n"); - usart1.puts("1234567890\n"); - usart1.puts("1234567890\n"); - usart1.puts("1234567890\n"); - usart1.puts("1234567890\n"); - usart1.puts("1234567890\n"); - usart1.puts("1234567890\n"); + sei(); + hal::usart::usart1.puts("1234567890\n"); + hal::usart::usart1.puts("1234567890\n"); + hal::usart::usart1.puts("1234567890\n"); + hal::usart::usart1.puts("1234567890\n"); + hal::usart::usart1.puts("1234567890\n"); + hal::usart::usart1.puts("1234567890\n"); + hal::usart::usart1.puts("1234567890\n"); + hal::usart::usart1.puts("1234567890\n"); + hal::usart::usart1.puts("1234567890\n"); + hal::usart::usart1.puts("1234567890\n"); + hal::usart::usart1.puts("1234567890\n"); } /// One-time setup of HW and SW components @@ -76,12 +74,12 @@ void setup() { // @@TODO if the shift register doesn't work we really can't signalize anything, only internal variables will be accessible if the UART works - USART::USART_InitTypeDef usart_conf = { + hal::usart::USART::USART_InitTypeDef usart_conf = { .rx_pin = gpio::GPIO_pin(GPIOD, 2), .tx_pin = gpio::GPIO_pin(GPIOD, 3), .baudrate = 115200, - usart1.Init(&usart_conf); - + }; + hal::usart::usart1.Init(&usart_conf); leds.SetMode(3, false, modules::leds::Mode::on); // shr::Send(leds.Step(0)); @@ -115,8 +113,8 @@ void ProcessRequestMsg(const modules::protocol::RequestMsg &rq) { /// @returns true if a request was successfully finished bool CheckMsgs() { using mpd = modules::protocol::DecodeStatus; - while (!uart.ReadEmpty()) { - switch (protocol.DecodeRequest(uart.Read())) { + while (!hal::usart::usart1.ReadEmpty()) { + switch (protocol.DecodeRequest(hal::usart::usart1.Read())) { case mpd::MessageCompleted: // process the input message return true; From acc33bfacb6686cb39fa7d04ee7b5d3333fc19d0 Mon Sep 17 00:00:00 2001 From: "D.R.racer" Date: Tue, 25 May 2021 12:23:29 +0200 Subject: [PATCH 3/7] Do not indent namespaces --- .clang-format | 2 +- src/modules/leds.h | 147 +++++++++++++++++---------------------------- 2 files changed, 55 insertions(+), 94 deletions(-) diff --git a/.clang-format b/.clang-format index a2d537b..65fa90c 100644 --- a/.clang-format +++ b/.clang-format @@ -84,7 +84,7 @@ KeepEmptyLinesAtTheStartOfBlocks: true MacroBlockBegin: '' MacroBlockEnd: '' MaxEmptyLinesToKeep: 1 -NamespaceIndentation: Inner +NamespaceIndentation: None ObjCBinPackProtocolList: Auto ObjCBlockIndentWidth: 4 ObjCSpaceAfterProperty: true diff --git a/src/modules/leds.h b/src/modules/leds.h index d3168b4..057113e 100644 --- a/src/modules/leds.h +++ b/src/modules/leds.h @@ -8,112 +8,73 @@ /// A typical scenario in the past was visualization of error states. /// The combination of colors with blinking frequency had a specific meaning. /// -/// We'd like to drive each pair? separately, which includes: -/// - blinking (none/slow/fast) -/// - what shall blink (red/green/both-at-once/both-interlaced) -/// /// The physical connection is not important on this level (i.e. how and what shall be sent into the shift registers) namespace modules { namespace leds { - /// Mode of LED - /// blink0 and blink1 allow for interlaced blinking of LEDs (one is on and the other off) - enum Mode { - off, - on, - blink0, ///< start blinking at even periods - blink1 ///< start blinking at odd periods +/// Mode of LED +/// blink0 and blink1 allow for interlaced blinking of LEDs (one is on and the other off) +enum Mode { + off, + on, + blink0, ///< start blinking at even periods + blink1 ///< start blinking at odd periods +}; + +/// a single LED +class LED { +public: + constexpr inline LED() = default; + void SetMode(Mode mode); + + /// @returns true if the LED shines + bool Step(bool oddPeriod); + inline bool On() const { return state.on; } + +private: + struct State { + uint8_t on : 1; + uint8_t mode : 2; + constexpr inline State() + : on(0) + , mode(Mode::off) {} }; - /// a single LED - class LED { - public: - constexpr inline LED() = default; - void SetMode(Mode mode); + State state; +}; - /// @returns true if the LED shines - bool Step(bool oddPeriod); - inline bool On() const { return state.on; } +/// main LED API +class LEDs { +public: + constexpr inline LEDs() + : ms(0) {}; - private: - struct State { - uint8_t on : 1; - uint8_t mode : 2; - constexpr inline State() - : on(0) - , mode(Mode::off) {} - }; + /// step LED automaton + /// @returns statuses of LEDs - one bit per LED and 1 = on, 0 = off + uint16_t Step(uint8_t delta_ms); - State state; - }; + inline constexpr uint8_t LedPairsCount() const { return ledPairs; } - /// main LED API - class LEDs { - public: - constexpr inline LEDs() - : ms(0) {}; + inline void SetMode(uint8_t slot, bool red, Mode mode) { + SetMode(slot * 2 + red, mode); + } + inline void SetMode(uint8_t index, Mode mode) { + leds[index].SetMode(mode); + } - /// step LED automaton - /// @returns statuses of LEDs - one bit per LED and 1 = on, 0 = off - uint16_t Step(uint8_t delta_ms); + inline bool LedOn(uint8_t index) const { + return leds[index].On(); + } + inline bool LedOn(uint8_t slot, bool red) const { + return leds[slot * 2 + red].On(); + } - inline constexpr uint8_t LedPairsCount() const { return ledPairs; } - - inline void SetMode(uint8_t slot, bool red, Mode mode) { - SetMode(slot * 2 + red, mode); - } - inline void SetMode(uint8_t index, Mode mode) { - leds[index].SetMode(mode); - } - - inline bool LedOn(uint8_t index) const { - return leds[index].On(); - } - inline bool LedOn(uint8_t slot, bool red) const { - return leds[slot * 2 + red].On(); - } - - private: - constexpr static const uint8_t ledPairs = 5; - LED leds[ledPairs * 2]; - uint16_t ms; - }; - - //// asi nechame moznost jednu blikat rychle a druhou pomalu - - //// tohle ale nevyresi, ze jedna LED ma blikat a druha svitit - - //// problem je, ze zapnuti/vypnuti led je blink none ... mozna by to chtelo blink none-on a none-off - //// jaka bude mnozina rezimu? - //// green-off green-on green-blink-slow green-blink-fast - //// red-off + + + + - //// red-on + + + + - //// red-blink-slow + + 2 x - //// red-blink-fast + + x 2 - //// - //// rezim 2 muze mit jeste varianty - blink-sync, blink-interlaced, coz v podstate znamena rezimy: - //// blink-slow1, blink-slow2, blink-fast1, blink-fast2, pricemz nemaji smysl kombinace: - //// blink-slow + blink-fast - - //// revize - //// v aktualnim FW led jen blikaji jednou rychlosti - //// takze to spis udelam jako pole LED, pricemz cervene budou na sudych a zelene na lichych indexech - //// stav ledky bude: off, on, blink0 a blink1 - //// blink0 znamena zacni pocitat blikaci interval od sude periody - //// blink1 od liche - //// tim pujde zmodelovat jak sync tak async blikani - //// Dale je otazka, jestli chceme rychle a pomale blikani... asi ne, kdyz bude report na LCD tiskarny - - //// Dale castecne souvisejici - //// Start MMU by mel reportovat progress, pricemz aktualni stav je takovy, ze nabihaji LED a kazda neco znamena - //// Tiskarna by klidne mohla posilat Q a dostavat odpoved ve stylu "starting" a progress/state code - //// - //// Mozna taky budeme potrebovat nastavovat ruzne vnitrni promenne (treba use slots 1-3 namisto 0-4), takze operace SetVar a k tomu obracena GetVar - //// Dava mi smysl modifikovat protokol V jako getvar (zamerne ne G, aby to nekolidovalo s gcodes) a S jako setvar - //// cislo je index variable a odpovi se A_hodnota, cili accepted a hodnota odpovedi - - //void SetLEDs(uint8_t slot0, uint8_t slot1, uint8_t slot2, uint8_t slot3, uint8_t slot4); +private: + constexpr static const uint8_t ledPairs = 5; + LED leds[ledPairs * 2]; + uint16_t ms; +}; } // namespace LEDs } // namespace modules From 9226230fd50f59048f25fc9fc956aef72a7da5fb Mon Sep 17 00:00:00 2001 From: "D.R.racer" Date: Tue, 25 May 2021 12:24:19 +0200 Subject: [PATCH 4/7] Reformat sources to fit the new namespace formatting rules --- src/hal/adc.cpp | 2 +- src/hal/adc.h | 4 +- src/hal/avr/cpu.cpp | 4 +- src/hal/avr/usart.cpp | 118 ++++---- src/hal/cpu.h | 4 +- src/hal/eeprom.h | 8 +- src/hal/gpio.h | 118 ++++---- src/hal/spi.h | 62 ++-- src/hal/timers.h | 8 +- src/hal/usart.h | 174 +++++------ src/hal/watchdog.h | 6 +- src/modules/buttons.cpp | 114 ++++---- src/modules/buttons.h | 98 +++---- src/modules/leds.cpp | 72 ++--- src/modules/protocol.cpp | 368 ++++++++++++------------ src/modules/protocol.h | 254 ++++++++-------- tests/unit/modules/buttons/stub_adc.cpp | 38 +-- tests/unit/modules/buttons/stub_adc.h | 4 +- 18 files changed, 728 insertions(+), 728 deletions(-) diff --git a/src/hal/adc.cpp b/src/hal/adc.cpp index c97b9c3..5089d7d 100644 --- a/src/hal/adc.cpp +++ b/src/hal/adc.cpp @@ -3,7 +3,7 @@ namespace hal { namespace adc { - uint16_t ReadADC(uint8_t adc) { return 0; } +uint16_t ReadADC(uint8_t adc) { return 0; } } // namespace adc } // namespace hal diff --git a/src/hal/adc.h b/src/hal/adc.h index cd9cef6..9116dff 100644 --- a/src/hal/adc.h +++ b/src/hal/adc.h @@ -6,8 +6,8 @@ namespace hal { namespace adc { - /// ADC access routines - uint16_t ReadADC(uint8_t adc); +/// ADC access routines +uint16_t ReadADC(uint8_t adc); } // namespace adc } // namespace hal diff --git a/src/hal/avr/cpu.cpp b/src/hal/avr/cpu.cpp index 991342c..96d539d 100644 --- a/src/hal/avr/cpu.cpp +++ b/src/hal/avr/cpu.cpp @@ -3,8 +3,8 @@ namespace hal { namespace cpu { - void Init() { - } +void Init() { +} } // namespace CPU } // namespace hal diff --git a/src/hal/avr/usart.cpp b/src/hal/avr/usart.cpp index 1dac360..436bbe7 100644 --- a/src/hal/avr/usart.cpp +++ b/src/hal/avr/usart.cpp @@ -4,72 +4,72 @@ namespace hal { namespace usart { - USART usart1; +USART usart1; - uint8_t USART::Read() { - uint8_t c = 0; - rx_buf.ConsumeFirst(c); - return c; +uint8_t USART::Read() { + uint8_t c = 0; + rx_buf.ConsumeFirst(c); + return c; +} + +void USART::Write(uint8_t c) { + _written = true; + // If the buffer and the data register is empty, just write the byte + // to the data register and be done. This shortcut helps + // significantly improve the effective datarate at high (> + // 500kbit/s) bitrates, where interrupt overhead becomes a slowdown. + if (tx_buf.IsEmpty() && (husart->UCSRxA & (1 << 5))) { + husart->UDRx = c; + husart->UCSRxA |= (1 << 6); + return; } - void USART::Write(uint8_t c) { - _written = true; - // If the buffer and the data register is empty, just write the byte - // to the data register and be done. This shortcut helps - // significantly improve the effective datarate at high (> - // 500kbit/s) bitrates, where interrupt overhead becomes a slowdown. - if (tx_buf.IsEmpty() && (husart->UCSRxA & (1 << 5))) { - husart->UDRx = c; - husart->UCSRxA |= (1 << 6); - return; - } - - // If the output buffer is full, there's nothing for it other than to - // wait for the interrupt handler to empty it a bit - while (!tx_buf.push_back_DontRewrite(c)) { - if (bit_is_clear(SREG, SREG_I)) { - // Interrupts are disabled, so we'll have to poll the data - // register empty flag ourselves. If it is set, pretend an - // interrupt has happened and call the handler to free up - // space for us. - if (husart->UCSRxA & (1 << 5)) { - ISR_UDRE(); - } - } else { - // nop, the interrupt handler will free up space for us + // If the output buffer is full, there's nothing for it other than to + // wait for the interrupt handler to empty it a bit + while (!tx_buf.push_back_DontRewrite(c)) { + if (bit_is_clear(SREG, SREG_I)) { + // Interrupts are disabled, so we'll have to poll the data + // register empty flag ourselves. If it is set, pretend an + // interrupt has happened and call the handler to free up + // space for us. + if (husart->UCSRxA & (1 << 5)) { + ISR_UDRE(); } - } - - husart->UCSRxB |= (1 << 5); //enable UDRE interrupt - } - - void USART::Flush() { - // If we have never written a byte, no need to flush. This special - // case is needed since there is no way to force the TXC (transmit - // complete) bit to 1 during initialization - if (!_written) { - return; - } - - while ((husart->UCSRxB & (1 << 5)) || ~(husart->UCSRxA & (1 << 6))) { - if (bit_is_clear(SREG, SREG_I) && (husart->UCSRxB & (1 << 5))) - // Interrupts are globally disabled, but the DR empty - // interrupt should be enabled, so poll the DR empty flag to - // prevent deadlock - if (husart->UCSRxA & (1 << 5)) { - ISR_UDRE(); - } - } - // If we get here, nothing is queued anymore (DRIE is disabled) and - // the hardware finished tranmission (TXC is set). - } - - void USART::puts(const char *str) { - while (*str) { - Write(*str++); + } else { + // nop, the interrupt handler will free up space for us } } + husart->UCSRxB |= (1 << 5); //enable UDRE interrupt +} + +void USART::Flush() { + // If we have never written a byte, no need to flush. This special + // case is needed since there is no way to force the TXC (transmit + // complete) bit to 1 during initialization + if (!_written) { + return; + } + + while ((husart->UCSRxB & (1 << 5)) || ~(husart->UCSRxA & (1 << 6))) { + if (bit_is_clear(SREG, SREG_I) && (husart->UCSRxB & (1 << 5))) + // Interrupts are globally disabled, but the DR empty + // interrupt should be enabled, so poll the DR empty flag to + // prevent deadlock + if (husart->UCSRxA & (1 << 5)) { + ISR_UDRE(); + } + } + // If we get here, nothing is queued anymore (DRIE is disabled) and + // the hardware finished tranmission (TXC is set). +} + +void USART::puts(const char *str) { + while (*str) { + Write(*str++); + } +} + } // namespace usart } // namespace hal diff --git a/src/hal/cpu.h b/src/hal/cpu.h index 880cc79..9775408 100644 --- a/src/hal/cpu.h +++ b/src/hal/cpu.h @@ -5,8 +5,8 @@ namespace hal { namespace cpu { - /// CPU init routines (not really necessary for the AVR) - void Init(); +/// CPU init routines (not really necessary for the AVR) +void Init(); } // namespace cpu } // namespace hal diff --git a/src/hal/eeprom.h b/src/hal/eeprom.h index 094cd37..231eeff 100644 --- a/src/hal/eeprom.h +++ b/src/hal/eeprom.h @@ -3,10 +3,10 @@ namespace hal { namespace EEPROM { - /// EEPROM interface - void WriteByte(uint16_t addr, uint8_t value); - void UpdateByte(uint16_t addr, uint8_t value); - uint8_t ReadByte(uint16_t addr); +/// EEPROM interface +void WriteByte(uint16_t addr, uint8_t value); +void UpdateByte(uint16_t addr, uint8_t value); +uint8_t ReadByte(uint16_t addr); } // namespace EEPROM } // namespace hal diff --git a/src/hal/gpio.h b/src/hal/gpio.h index 4d473bf..aceaa17 100644 --- a/src/hal/gpio.h +++ b/src/hal/gpio.h @@ -5,72 +5,72 @@ namespace hal { namespace gpio { - struct GPIO_TypeDef { - volatile uint8_t PINx; - volatile uint8_t DDRx; - volatile uint8_t PORTx; - }; +struct GPIO_TypeDef { + volatile uint8_t PINx; + volatile uint8_t DDRx; + volatile uint8_t PORTx; +}; - enum class Mode : uint8_t { - input = 0, - output, - }; +enum class Mode : uint8_t { + input = 0, + output, +}; - enum class Pull : uint8_t { - none = 0, - up, - down, //not available on the AVR - }; +enum class Pull : uint8_t { + none = 0, + up, + down, //not available on the AVR +}; - enum class Level : uint8_t { - low = 0, - high, - }; +enum class Level : uint8_t { + low = 0, + high, +}; - struct GPIO_InitTypeDef { - Mode mode; - Pull pull; - Level level; - inline GPIO_InitTypeDef(Mode mode, Pull pull) - : mode(mode) - , pull(pull) {}; - inline GPIO_InitTypeDef(Mode mode, Level level) - : mode(mode) - , level(level) {}; - }; +struct GPIO_InitTypeDef { + Mode mode; + Pull pull; + Level level; + inline GPIO_InitTypeDef(Mode mode, Pull pull) + : mode(mode) + , pull(pull) {}; + inline GPIO_InitTypeDef(Mode mode, Level level) + : mode(mode) + , level(level) {}; +}; - struct GPIO_pin { - GPIO_TypeDef *const port; - const uint8_t pin; - inline GPIO_pin(GPIO_TypeDef *const port, const uint8_t pin) - : port(port) - , pin(pin) {}; - }; +struct GPIO_pin { + GPIO_TypeDef *const port; + const uint8_t pin; + inline GPIO_pin(GPIO_TypeDef *const port, const uint8_t pin) + : port(port) + , pin(pin) {}; +}; - __attribute__((always_inline)) inline void WritePin(const GPIO_pin portPin, Level level) { - if (level == Level::high) - portPin.port->PORTx |= (1 << portPin.pin); - else - portPin.port->PORTx &= ~(1 << portPin.pin); - } - - __attribute__((always_inline)) inline Level ReadPin(const GPIO_pin portPin) { - return (Level)(portPin.port->PINx & (1 << portPin.pin)); - } - - __attribute__((always_inline)) inline void TogglePin(const GPIO_pin portPin) { - portPin.port->PINx |= (1 << portPin.pin); - } - - __attribute__((always_inline)) inline void Init(const GPIO_pin portPin, GPIO_InitTypeDef GPIO_Init) { - if (GPIO_Init.mode == Mode::output) { - WritePin(portPin, GPIO_Init.level); - portPin.port->DDRx |= (1 << portPin.pin); - } else { - portPin.port->DDRx &= ~(1 << portPin.pin); - WritePin(portPin, (Level)GPIO_Init.pull); - } +__attribute__((always_inline)) inline void WritePin(const GPIO_pin portPin, Level level) { + if (level == Level::high) + portPin.port->PORTx |= (1 << portPin.pin); + else + portPin.port->PORTx &= ~(1 << portPin.pin); +} + +__attribute__((always_inline)) inline Level ReadPin(const GPIO_pin portPin) { + return (Level)(portPin.port->PINx & (1 << portPin.pin)); +} + +__attribute__((always_inline)) inline void TogglePin(const GPIO_pin portPin) { + portPin.port->PINx |= (1 << portPin.pin); +} + +__attribute__((always_inline)) inline void Init(const GPIO_pin portPin, GPIO_InitTypeDef GPIO_Init) { + if (GPIO_Init.mode == Mode::output) { + WritePin(portPin, GPIO_Init.level); + portPin.port->DDRx |= (1 << portPin.pin); + } else { + portPin.port->DDRx &= ~(1 << portPin.pin); + WritePin(portPin, (Level)GPIO_Init.pull); } +} } } diff --git a/src/hal/spi.h b/src/hal/spi.h index 5e067d3..8b9546e 100644 --- a/src/hal/spi.h +++ b/src/hal/spi.h @@ -6,42 +6,42 @@ namespace hal { namespace spi { - struct SPI_TypeDef { - volatile uint8_t SPCRx; - volatile uint8_t SPSRx; - volatile uint8_t SPDRx; - }; +struct SPI_TypeDef { + volatile uint8_t SPCRx; + volatile uint8_t SPSRx; + volatile uint8_t SPDRx; +}; - struct SPI_InitTypeDef { - hal::gpio::GPIO_pin miso_pin; - hal::gpio::GPIO_pin mosi_pin; - hal::gpio::GPIO_pin sck_pin; - hal::gpio::GPIO_pin ss_pin; - uint8_t prescaler; - uint8_t cpha; - uint8_t cpol; - }; +struct SPI_InitTypeDef { + hal::gpio::GPIO_pin miso_pin; + hal::gpio::GPIO_pin mosi_pin; + hal::gpio::GPIO_pin sck_pin; + hal::gpio::GPIO_pin ss_pin; + uint8_t prescaler; + uint8_t cpha; + uint8_t cpol; +}; - __attribute__((always_inline)) inline void Init(SPI_TypeDef *const hspi, SPI_InitTypeDef *const conf) { - using namespace hal; - gpio::Init(conf->miso_pin, gpio::GPIO_InitTypeDef(gpio::Mode::input, gpio::Pull::none)); - gpio::Init(conf->mosi_pin, gpio::GPIO_InitTypeDef(gpio::Mode::output, gpio::Level::low)); - gpio::Init(conf->sck_pin, gpio::GPIO_InitTypeDef(gpio::Mode::output, gpio::Level::low)); - gpio::Init(conf->ss_pin, gpio::GPIO_InitTypeDef(gpio::Mode::output, gpio::Level::high)); //the AVR requires this pin to be an output for SPI master mode to work properly. +__attribute__((always_inline)) inline void Init(SPI_TypeDef *const hspi, SPI_InitTypeDef *const conf) { + using namespace hal; + gpio::Init(conf->miso_pin, gpio::GPIO_InitTypeDef(gpio::Mode::input, gpio::Pull::none)); + gpio::Init(conf->mosi_pin, gpio::GPIO_InitTypeDef(gpio::Mode::output, gpio::Level::low)); + gpio::Init(conf->sck_pin, gpio::GPIO_InitTypeDef(gpio::Mode::output, gpio::Level::low)); + gpio::Init(conf->ss_pin, gpio::GPIO_InitTypeDef(gpio::Mode::output, gpio::Level::high)); //the AVR requires this pin to be an output for SPI master mode to work properly. - const uint8_t spi2x = (conf->prescaler == 7) ? 0 : (conf->prescaler & 0x01); - const uint8_t spr = ((conf->prescaler - 1) >> 1) & 0x03; + const uint8_t spi2x = (conf->prescaler == 7) ? 0 : (conf->prescaler & 0x01); + const uint8_t spr = ((conf->prescaler - 1) >> 1) & 0x03; - hspi->SPCRx = (0 << SPIE) | (1 << SPE) | (0 << DORD) | (1 << MSTR) | ((conf->cpol & 0x01) << CPOL) | ((conf->cpha & 0x01) << CPHA) | (spr << SPR0); - hspi->SPSRx = (spi2x << SPI2X); - } + hspi->SPCRx = (0 << SPIE) | (1 << SPE) | (0 << DORD) | (1 << MSTR) | ((conf->cpol & 0x01) << CPOL) | ((conf->cpha & 0x01) << CPHA) | (spr << SPR0); + hspi->SPSRx = (spi2x << SPI2X); +} - __attribute__((always_inline)) inline uint8_t TxRx(SPI_TypeDef *const hspi, uint8_t val) { - hspi->SPDRx = val; - while (!(hspi->SPSRx & (1 << SPIF))) - ; - return hspi->SPDRx; - } +__attribute__((always_inline)) inline uint8_t TxRx(SPI_TypeDef *const hspi, uint8_t val) { + hspi->SPDRx = val; + while (!(hspi->SPSRx & (1 << SPIF))) + ; + return hspi->SPDRx; +} } } diff --git a/src/hal/timers.h b/src/hal/timers.h index f233616..8eba17b 100644 --- a/src/hal/timers.h +++ b/src/hal/timers.h @@ -5,10 +5,10 @@ namespace hal { namespace timers { - /// timers - void ConfigureTimer(uint8_t timer /* some config struct */); - void StartTimer(uint8_t timer); - void StopTimer(uint8_t timer); +/// timers +void ConfigureTimer(uint8_t timer /* some config struct */); +void StartTimer(uint8_t timer); +void StopTimer(uint8_t timer); } // namespace cpu } // namespace hal diff --git a/src/hal/usart.h b/src/hal/usart.h index 080e5cc..c6f8612 100644 --- a/src/hal/usart.h +++ b/src/hal/usart.h @@ -10,95 +10,95 @@ namespace hal { namespace usart { - class USART { - public: - struct USART_TypeDef { - volatile uint8_t UCSRxA; - volatile uint8_t UCSRxB; - volatile uint8_t UCSRxC; - volatile uint8_t UCSRxD; - volatile uint16_t UBRRx; - volatile uint8_t UDRx; - }; - - struct USART_InitTypeDef { - hal::gpio::GPIO_pin rx_pin; - hal::gpio::GPIO_pin tx_pin; - uint32_t baudrate; - }; - - /// @returns current character from the UART without extracting it from the read buffer - uint8_t Peek() const { - return rx_buf.GetFirstIfAble(); - } - /// @returns true if there are no bytes to be read - bool ReadEmpty() const { - return rx_buf.IsEmpty(); - } - /// @returns current character from the UART and extracts it from the read buffer - uint8_t Read(); - - /// @param c character to be pushed into the TX buffer (to be sent) - void Write(uint8_t c); - /// @param str c string to be sent. NL is appended - void puts(const char *str); - /// @returns true if there is at least one byte free in the TX buffer (i.e. some space to add a character to be sent) - bool CanWrite() const { - return tx_buf.CanPush(); - } - /// blocks until the TX buffer was successfully transmitted - void Flush(); - - /// Initializes USART interface - __attribute__((always_inline)) inline void Init(USART_InitTypeDef *const conf) { - gpio::Init(conf->rx_pin, gpio::GPIO_InitTypeDef(gpio::Mode::input, gpio::Level::low)); - gpio::Init(conf->tx_pin, gpio::GPIO_InitTypeDef(gpio::Mode::output, gpio::Level::low)); - husart->UBRRx = (((double)(F_CPU)) / (((double)(conf->baudrate)) * 8.0) - 1.0 + 0.5); - husart->UCSRxA = (1 << 1); // Set double baudrate setting. Clear all other status bits/flags - // husart->UCSRxC |= (1 << 3); // 2 stop bits. Preserve data size setting - husart->UCSRxD = 0; //disable hardware flow control. This register is reserved on all AVR devides with USART. - husart->UCSRxB = (1 << 3) | (1 << 4) | (1 << 7); // Turn on the transmission and reception circuitry and enable the RX interrupt - } - - /// implementation of the receive ISR's body - __attribute__((always_inline)) inline void ISR_RX() { - if (husart->UCSRxA & (1 << 4)) { - (void)husart->UDRx; - } else { - rx_buf.push_back_DontRewrite(husart->UDRx); - } - } - /// implementation of the transmit ISR's body - __attribute__((always_inline)) inline void ISR_UDRE() { - uint8_t c = 0; - tx_buf.ConsumeFirst(c); - husart->UDRx = c; - - // clear the TXC bit -- "can be cleared by writing a one to its bit - // location". This makes sure flush() won't return until the bytes - // actually got written - husart->UCSRxA |= (1 << 6); - - if (tx_buf.IsEmpty()) - husart->UCSRxB &= ~(1 << 5); // disable UDRE interrupt - } - - USART() = default; - void Init(USART_TypeDef *conf) { - husart = conf; - } - - private: - // IO base address - USART_TypeDef *husart; - bool _written; - - CircleBuffer tx_buf; - CircleBuffer rx_buf; +class USART { +public: + struct USART_TypeDef { + volatile uint8_t UCSRxA; + volatile uint8_t UCSRxB; + volatile uint8_t UCSRxC; + volatile uint8_t UCSRxD; + volatile uint16_t UBRRx; + volatile uint8_t UDRx; }; - /// beware - normally we'd make a singleton, but avr-gcc generates suboptimal code for them, therefore we only keep this extern variable - extern USART usart1; + struct USART_InitTypeDef { + hal::gpio::GPIO_pin rx_pin; + hal::gpio::GPIO_pin tx_pin; + uint32_t baudrate; + }; + + /// @returns current character from the UART without extracting it from the read buffer + uint8_t Peek() const { + return rx_buf.GetFirstIfAble(); + } + /// @returns true if there are no bytes to be read + bool ReadEmpty() const { + return rx_buf.IsEmpty(); + } + /// @returns current character from the UART and extracts it from the read buffer + uint8_t Read(); + + /// @param c character to be pushed into the TX buffer (to be sent) + void Write(uint8_t c); + /// @param str c string to be sent. NL is appended + void puts(const char *str); + /// @returns true if there is at least one byte free in the TX buffer (i.e. some space to add a character to be sent) + bool CanWrite() const { + return tx_buf.CanPush(); + } + /// blocks until the TX buffer was successfully transmitted + void Flush(); + + /// Initializes USART interface + __attribute__((always_inline)) inline void Init(USART_InitTypeDef *const conf) { + gpio::Init(conf->rx_pin, gpio::GPIO_InitTypeDef(gpio::Mode::input, gpio::Level::low)); + gpio::Init(conf->tx_pin, gpio::GPIO_InitTypeDef(gpio::Mode::output, gpio::Level::low)); + husart->UBRRx = (((double)(F_CPU)) / (((double)(conf->baudrate)) * 8.0) - 1.0 + 0.5); + husart->UCSRxA = (1 << 1); // Set double baudrate setting. Clear all other status bits/flags + // husart->UCSRxC |= (1 << 3); // 2 stop bits. Preserve data size setting + husart->UCSRxD = 0; //disable hardware flow control. This register is reserved on all AVR devides with USART. + husart->UCSRxB = (1 << 3) | (1 << 4) | (1 << 7); // Turn on the transmission and reception circuitry and enable the RX interrupt + } + + /// implementation of the receive ISR's body + __attribute__((always_inline)) inline void ISR_RX() { + if (husart->UCSRxA & (1 << 4)) { + (void)husart->UDRx; + } else { + rx_buf.push_back_DontRewrite(husart->UDRx); + } + } + /// implementation of the transmit ISR's body + __attribute__((always_inline)) inline void ISR_UDRE() { + uint8_t c = 0; + tx_buf.ConsumeFirst(c); + husart->UDRx = c; + + // clear the TXC bit -- "can be cleared by writing a one to its bit + // location". This makes sure flush() won't return until the bytes + // actually got written + husart->UCSRxA |= (1 << 6); + + if (tx_buf.IsEmpty()) + husart->UCSRxB &= ~(1 << 5); // disable UDRE interrupt + } + + USART() = default; + void Init(USART_TypeDef *conf) { + husart = conf; + } + +private: + // IO base address + USART_TypeDef *husart; + bool _written; + + CircleBuffer tx_buf; + CircleBuffer rx_buf; +}; + +/// beware - normally we'd make a singleton, but avr-gcc generates suboptimal code for them, therefore we only keep this extern variable +extern USART usart1; } // namespace usart } // namespace hal diff --git a/src/hal/watchdog.h b/src/hal/watchdog.h index de264a2..850589d 100644 --- a/src/hal/watchdog.h +++ b/src/hal/watchdog.h @@ -5,9 +5,9 @@ namespace hal { namespace watchdog { - /// watchdog interface - void ConfigureWatchDog(uint16_t period); - void ResetWatchDog(); +/// watchdog interface +void ConfigureWatchDog(uint16_t period); +void ResetWatchDog(); } // namespace watchdog } // namespace hal diff --git a/src/modules/buttons.cpp b/src/modules/buttons.cpp index 7ce76e1..94a0b6f 100644 --- a/src/modules/buttons.cpp +++ b/src/modules/buttons.cpp @@ -3,69 +3,69 @@ namespace modules { namespace buttons { - uint16_t Buttons::tmpTiming = 0; +uint16_t Buttons::tmpTiming = 0; - // original idea from: https://www.eeweb.com/debouncing-push-buttons-using-a-state-machine-approach - void Button::Step(uint16_t time, bool press) { - switch (f.state) { - case State::Waiting: - if (press) { - f.state = State::Detected; - timeLastChange = time; - f.tmp = press; - } - break; - case State::Detected: - if (f.tmp == press) { - if (time - timeLastChange > debounce) { - f.state = State::WaitForRelease; - } - } else { - f.state = State::Waiting; - } - break; - case State::WaitForRelease: - if (!press) { - f.state = State::Update; - } - break; - case State::Update: - f.state = State::Waiting; +// original idea from: https://www.eeweb.com/debouncing-push-buttons-using-a-state-machine-approach +void Button::Step(uint16_t time, bool press) { + switch (f.state) { + case State::Waiting: + if (press) { + f.state = State::Detected; timeLastChange = time; - f.tmp = false; - break; - default: + f.tmp = press; + } + break; + case State::Detected: + if (f.tmp == press) { + if (time - timeLastChange > debounce) { + f.state = State::WaitForRelease; + } + } else { f.state = State::Waiting; - timeLastChange = time; - f.tmp = false; } - } - - int8_t Buttons::Sample(uint16_t rawADC) { - // decode 3 buttons' levels from one ADC - // Button 1 - 0 - // Button 2 - 344 - // Button 3 - 516 - // Doesn't handle multiple pressed buttons at once - - if (rawADC < 10) - return 0; - else if (rawADC > 320 && rawADC < 360) - return 1; - else if (rawADC > 500 && rawADC < 530) - return 2; - return -1; - } - - void Buttons::Step(uint16_t rawADC) { - // @@TODO temporary timing - ++tmpTiming; - int8_t currentState = Sample(rawADC); - for (uint_fast8_t b = 0; b < N; ++b) { - // this button was pressed if b == currentState, released otherwise - buttons[b].Step(tmpTiming, b == currentState); + break; + case State::WaitForRelease: + if (!press) { + f.state = State::Update; } + break; + case State::Update: + f.state = State::Waiting; + timeLastChange = time; + f.tmp = false; + break; + default: + f.state = State::Waiting; + timeLastChange = time; + f.tmp = false; } +} + +int8_t Buttons::Sample(uint16_t rawADC) { + // decode 3 buttons' levels from one ADC + // Button 1 - 0 + // Button 2 - 344 + // Button 3 - 516 + // Doesn't handle multiple pressed buttons at once + + if (rawADC < 10) + return 0; + else if (rawADC > 320 && rawADC < 360) + return 1; + else if (rawADC > 500 && rawADC < 530) + return 2; + return -1; +} + +void Buttons::Step(uint16_t rawADC) { + // @@TODO temporary timing + ++tmpTiming; + int8_t currentState = Sample(rawADC); + for (uint_fast8_t b = 0; b < N; ++b) { + // this button was pressed if b == currentState, released otherwise + buttons[b].Step(tmpTiming, b == currentState); + } +} } // namespace buttons } // namespace modules diff --git a/src/modules/buttons.h b/src/modules/buttons.h index d20260e..6f9fc53 100644 --- a/src/modules/buttons.h +++ b/src/modules/buttons.h @@ -9,67 +9,67 @@ namespace modules { namespace buttons { - struct Button { - inline constexpr Button() - : timeLastChange(0) {} +struct Button { + inline constexpr Button() + : timeLastChange(0) {} - /// @returns true if button is currently considered as pressed - inline bool Pressed() const { return f.state == State::WaitForRelease; } + /// @returns true if button is currently considered as pressed + inline bool Pressed() const { return f.state == State::WaitForRelease; } - /// State machine stepping routine - void Step(uint16_t time, bool press); + /// State machine stepping routine + void Step(uint16_t time, bool press); - private: - /// time interval for debouncing @@TODO specify units - constexpr static const uint16_t debounce = 100; +private: + /// time interval for debouncing @@TODO specify units + constexpr static const uint16_t debounce = 100; - /// States of the debouncing automaton - /// Intentionally not modeled as an enum class - /// as it would impose additional casts which do not play well with the struct Flags - /// and would make the code less readable - enum State { Waiting = 0, - Detected, - WaitForRelease, - Update }; + /// States of the debouncing automaton + /// Intentionally not modeled as an enum class + /// as it would impose additional casts which do not play well with the struct Flags + /// and would make the code less readable + enum State { Waiting = 0, + Detected, + WaitForRelease, + Update }; - /// The sole purpose of this data struct is to save RAM by compressing several flags into one byte on the AVR - struct Flags { - uint8_t state : 2; ///< state of the button - uint8_t tmp : 1; ///< temporary state of button before the debouncing state machine finishes - inline constexpr Flags() - : state(State::Waiting) - , tmp(false) {} - }; - - /// Flags and state of the debouncing automaton - Flags f; - - /// Timestamp of the last change of ADC state for this button - uint16_t timeLastChange; + /// The sole purpose of this data struct is to save RAM by compressing several flags into one byte on the AVR + struct Flags { + uint8_t state : 2; ///< state of the button + uint8_t tmp : 1; ///< temporary state of button before the debouncing state machine finishes + inline constexpr Flags() + : state(State::Waiting) + , tmp(false) {} }; - class Buttons { - constexpr static const uint8_t N = 3; ///< number of buttons currently supported - constexpr static const uint8_t adc = 1; ///< ADC index - will be some define or other constant later on - static uint16_t tmpTiming; ///< subject to removal when we have timers implemented - now used for the unit tests + /// Flags and state of the debouncing automaton + Flags f; - public: - inline constexpr Buttons() = default; + /// Timestamp of the last change of ADC state for this button + uint16_t timeLastChange; +}; - /// State machine step - reads the ADC, processes debouncing, updates states of individual buttons - void Step(uint16_t rawADC); +class Buttons { + constexpr static const uint8_t N = 3; ///< number of buttons currently supported + constexpr static const uint8_t adc = 1; ///< ADC index - will be some define or other constant later on + static uint16_t tmpTiming; ///< subject to removal when we have timers implemented - now used for the unit tests - /// @return true if button at index is pressed - /// @@TODO add range checking if necessary - inline bool ButtonPressed(uint8_t index) const { return buttons[index].Pressed(); } +public: + inline constexpr Buttons() = default; - private: - Button buttons[N]; + /// State machine step - reads the ADC, processes debouncing, updates states of individual buttons + void Step(uint16_t rawADC); - /// Call to the ADC and decode its output into a button index - /// @returns index of the button pressed or -1 in case no button is pressed - static int8_t Sample(uint16_t rawADC); - }; + /// @return true if button at index is pressed + /// @@TODO add range checking if necessary + inline bool ButtonPressed(uint8_t index) const { return buttons[index].Pressed(); } + +private: + Button buttons[N]; + + /// Call to the ADC and decode its output into a button index + /// @returns index of the button pressed or -1 in case no button is pressed + static int8_t Sample(uint16_t rawADC); +}; } // namespace buttons } // namespace modules diff --git a/src/modules/leds.cpp b/src/modules/leds.cpp index c994d42..4787e92 100644 --- a/src/modules/leds.cpp +++ b/src/modules/leds.cpp @@ -3,48 +3,48 @@ namespace modules { namespace leds { - void LED::SetMode(Mode mode) { - state.mode = mode; - // set initial state of LEDs correctly - transition from one mode to another - switch (state.mode) { - case Mode::blink1: - case Mode::off: - state.on = 0; - break; +void LED::SetMode(Mode mode) { + state.mode = mode; + // set initial state of LEDs correctly - transition from one mode to another + switch (state.mode) { + case Mode::blink1: + case Mode::off: + state.on = 0; + break; - case Mode::blink0: - case Mode::on: - state.on = 1; - break; - default: - break; - } + case Mode::blink0: + case Mode::on: + state.on = 1; + break; + default: + break; } +} - bool LED::Step(bool oddPeriod) { - switch (state.mode) { - // on and off don't change while stepping - case Mode::blink0: - state.on = oddPeriod; - break; - case Mode::blink1: - state.on = !oddPeriod; - break; - default: // do nothing - break; - } +bool LED::Step(bool oddPeriod) { + switch (state.mode) { + // on and off don't change while stepping + case Mode::blink0: + state.on = oddPeriod; + break; + case Mode::blink1: + state.on = !oddPeriod; + break; + default: // do nothing + break; } +} - uint16_t LEDs::Step(uint8_t delta_ms) { - ms += delta_ms; - bool oddPeriod = ((ms / 1000U) & 0x01U) != 0; - uint16_t result = 0; - for (uint8_t i = 0; i < ledPairs * 2; ++i) { - result <<= 1; - result |= leds[i].Step(oddPeriod); - } - return result; +uint16_t LEDs::Step(uint8_t delta_ms) { + ms += delta_ms; + bool oddPeriod = ((ms / 1000U) & 0x01U) != 0; + uint16_t result = 0; + for (uint8_t i = 0; i < ledPairs * 2; ++i) { + result <<= 1; + result |= leds[i].Step(oddPeriod); } + return result; +} } // namespace leds } // namespace modules diff --git a/src/modules/protocol.cpp b/src/modules/protocol.cpp index 356a30e..a230fd3 100644 --- a/src/modules/protocol.cpp +++ b/src/modules/protocol.cpp @@ -13,176 +13,197 @@ namespace modules { namespace protocol { - // decoding automaton - // states: input -> transition into state - // Code QTLMUXPSBEWK -> msgcode - // \n ->start - // * ->error - // error \n ->start - // * ->error - // msgcode 0-9 ->msgvalue - // * ->error - // msgvalue 0-9 ->msgvalue - // \n ->start successfully accepted command +// decoding automaton +// states: input -> transition into state +// Code QTLMUXPSBEWK -> msgcode +// \n ->start +// * ->error +// error \n ->start +// * ->error +// msgcode 0-9 ->msgvalue +// * ->error +// msgvalue 0-9 ->msgvalue +// \n ->start successfully accepted command - DecodeStatus Protocol::DecodeRequest(uint8_t c) { - switch (rqState) { - case RequestStates::Code: - switch (c) { - case 'Q': - case 'T': - case 'L': - case 'M': - case 'U': - case 'X': - case 'P': - case 'S': - case 'B': - case 'E': - case 'W': - case 'K': - requestMsg.code = (RequestMsgCodes)c; - requestMsg.value = 0; - rqState = RequestStates::Value; - return DecodeStatus::NeedMoreData; - default: - requestMsg.code = RequestMsgCodes::unknown; - rqState = RequestStates::Error; - return DecodeStatus::Error; - } - case RequestStates::Value: - if (c >= '0' && c <= '9') { - requestMsg.value *= 10; - requestMsg.value += c - '0'; - return DecodeStatus::NeedMoreData; - } else if (c == '\n') { - rqState = RequestStates::Code; - return DecodeStatus::MessageCompleted; - } else { - requestMsg.code = RequestMsgCodes::unknown; - rqState = RequestStates::Error; - return DecodeStatus::Error; - } - default: //case error: - if (c == '\n') { - rqState = RequestStates::Code; - return DecodeStatus::MessageCompleted; - } else { - requestMsg.code = RequestMsgCodes::unknown; - rqState = RequestStates::Error; - return DecodeStatus::Error; - } +DecodeStatus Protocol::DecodeRequest(uint8_t c) { + switch (rqState) { + case RequestStates::Code: + switch (c) { + case 'Q': + case 'T': + case 'L': + case 'M': + case 'U': + case 'X': + case 'P': + case 'S': + case 'B': + case 'E': + case 'W': + case 'K': + requestMsg.code = (RequestMsgCodes)c; + requestMsg.value = 0; + rqState = RequestStates::Value; + return DecodeStatus::NeedMoreData; + default: + requestMsg.code = RequestMsgCodes::unknown; + rqState = RequestStates::Error; + return DecodeStatus::Error; + } + case RequestStates::Value: + if (c >= '0' && c <= '9') { + requestMsg.value *= 10; + requestMsg.value += c - '0'; + return DecodeStatus::NeedMoreData; + } else if (c == '\n') { + rqState = RequestStates::Code; + return DecodeStatus::MessageCompleted; + } else { + requestMsg.code = RequestMsgCodes::unknown; + rqState = RequestStates::Error; + return DecodeStatus::Error; + } + default: //case error: + if (c == '\n') { + rqState = RequestStates::Code; + return DecodeStatus::MessageCompleted; + } else { + requestMsg.code = RequestMsgCodes::unknown; + rqState = RequestStates::Error; + return DecodeStatus::Error; } } +} - uint8_t Protocol::EncodeRequest(const RequestMsg &msg, uint8_t *txbuff) { - txbuff[0] = (uint8_t)msg.code; - txbuff[1] = msg.value + '0'; - txbuff[2] = '\n'; - return 3; - } +uint8_t Protocol::EncodeRequest(const RequestMsg &msg, uint8_t *txbuff) { + txbuff[0] = (uint8_t)msg.code; + txbuff[1] = msg.value + '0'; + txbuff[2] = '\n'; + return 3; +} - DecodeStatus Protocol::DecodeResponse(uint8_t c) { - switch (rspState) { - case ResponseStates::RequestCode: - switch (c) { - case 'Q': - case 'T': - case 'L': - case 'M': - case 'U': - case 'X': - case 'P': - case 'S': - case 'B': - case 'E': - case 'W': - case 'K': - responseMsg.request.code = (RequestMsgCodes)c; - responseMsg.request.value = 0; - rspState = ResponseStates::RequestValue; - return DecodeStatus::NeedMoreData; - default: - rspState = ResponseStates::Error; - return DecodeStatus::Error; - } - case ResponseStates::RequestValue: - if (c >= '0' && c <= '9') { - responseMsg.request.value *= 10; - responseMsg.request.value += c - '0'; - return DecodeStatus::NeedMoreData; - } else if (c == ' ') { - rspState = ResponseStates::ParamCode; - return DecodeStatus::NeedMoreData; - } else { - rspState = ResponseStates::Error; - return DecodeStatus::Error; - } - case ResponseStates::ParamCode: - switch (c) { - case 'P': - case 'E': - case 'F': - case 'A': - case 'R': - rspState = ResponseStates::ParamValue; - responseMsg.paramCode = (ResponseMsgParamCodes)c; - responseMsg.paramValue = 0; - return DecodeStatus::NeedMoreData; - default: - responseMsg.paramCode = ResponseMsgParamCodes::unknown; - rspState = ResponseStates::Error; - return DecodeStatus::Error; - } - case ResponseStates::ParamValue: - if (c >= '0' && c <= '9') { - responseMsg.paramValue *= 10; - responseMsg.paramValue += c - '0'; - return DecodeStatus::NeedMoreData; - } else if (c == '\n') { - rspState = ResponseStates::RequestCode; - return DecodeStatus::MessageCompleted; - } else { - responseMsg.paramCode = ResponseMsgParamCodes::unknown; - rspState = ResponseStates::Error; - return DecodeStatus::Error; - } - default: //case error: - if (c == '\n') { - rspState = ResponseStates::RequestCode; - return DecodeStatus::MessageCompleted; - } else { - responseMsg.paramCode = ResponseMsgParamCodes::unknown; - return DecodeStatus::Error; - } +DecodeStatus Protocol::DecodeResponse(uint8_t c) { + switch (rspState) { + case ResponseStates::RequestCode: + switch (c) { + case 'Q': + case 'T': + case 'L': + case 'M': + case 'U': + case 'X': + case 'P': + case 'S': + case 'B': + case 'E': + case 'W': + case 'K': + responseMsg.request.code = (RequestMsgCodes)c; + responseMsg.request.value = 0; + rspState = ResponseStates::RequestValue; + return DecodeStatus::NeedMoreData; + default: + rspState = ResponseStates::Error; + return DecodeStatus::Error; + } + case ResponseStates::RequestValue: + if (c >= '0' && c <= '9') { + responseMsg.request.value *= 10; + responseMsg.request.value += c - '0'; + return DecodeStatus::NeedMoreData; + } else if (c == ' ') { + rspState = ResponseStates::ParamCode; + return DecodeStatus::NeedMoreData; + } else { + rspState = ResponseStates::Error; + return DecodeStatus::Error; + } + case ResponseStates::ParamCode: + switch (c) { + case 'P': + case 'E': + case 'F': + case 'A': + case 'R': + rspState = ResponseStates::ParamValue; + responseMsg.paramCode = (ResponseMsgParamCodes)c; + responseMsg.paramValue = 0; + return DecodeStatus::NeedMoreData; + default: + responseMsg.paramCode = ResponseMsgParamCodes::unknown; + rspState = ResponseStates::Error; + return DecodeStatus::Error; + } + case ResponseStates::ParamValue: + if (c >= '0' && c <= '9') { + responseMsg.paramValue *= 10; + responseMsg.paramValue += c - '0'; + return DecodeStatus::NeedMoreData; + } else if (c == '\n') { + rspState = ResponseStates::RequestCode; + return DecodeStatus::MessageCompleted; + } else { + responseMsg.paramCode = ResponseMsgParamCodes::unknown; + rspState = ResponseStates::Error; + return DecodeStatus::Error; + } + default: //case error: + if (c == '\n') { + rspState = ResponseStates::RequestCode; + return DecodeStatus::MessageCompleted; + } else { + responseMsg.paramCode = ResponseMsgParamCodes::unknown; + return DecodeStatus::Error; } } +} - uint8_t Protocol::EncodeResponseCmdAR(const RequestMsg &msg, ResponseMsgParamCodes ar, uint8_t *txbuff) { - txbuff[0] = (uint8_t)msg.code; - txbuff[1] = msg.value + '0'; - txbuff[2] = ' '; - txbuff[3] = (uint8_t)ar; - txbuff[4] = '\n'; - return 5; +uint8_t Protocol::EncodeResponseCmdAR(const RequestMsg &msg, ResponseMsgParamCodes ar, uint8_t *txbuff) { + txbuff[0] = (uint8_t)msg.code; + txbuff[1] = msg.value + '0'; + txbuff[2] = ' '; + txbuff[3] = (uint8_t)ar; + txbuff[4] = '\n'; + return 5; +} + +uint8_t Protocol::EncodeResponseReadFINDA(const RequestMsg &msg, uint8_t findaValue, uint8_t *txbuff) { + txbuff[0] = (uint8_t)msg.code; + txbuff[1] = msg.value + '0'; + txbuff[2] = ' '; + txbuff[3] = (uint8_t)ResponseMsgParamCodes::Accepted; + txbuff[4] = findaValue + '0'; + txbuff[5] = '\n'; + return 6; +} + +uint8_t Protocol::EncodeResponseVersion(const RequestMsg &msg, uint8_t value, uint8_t *txbuff) { + txbuff[0] = (uint8_t)msg.code; + txbuff[1] = msg.value + '0'; + txbuff[2] = ' '; + txbuff[3] = (uint8_t)ResponseMsgParamCodes::Accepted; + uint8_t *dst = txbuff + 4; + if (value < 10) { + *dst++ = value + '0'; + } else if (value < 100) { + *dst++ = value / 10 + '0'; + *dst++ = value % 10 + '0'; + } else { + *dst++ = value / 100 + '0'; + *dst++ = (value / 10) % 10 + '0'; + *dst++ = value % 10 + '0'; } + *dst = '\n'; + return dst - txbuff + 1; +} - uint8_t Protocol::EncodeResponseReadFINDA(const RequestMsg &msg, uint8_t findaValue, uint8_t *txbuff) { - txbuff[0] = (uint8_t)msg.code; - txbuff[1] = msg.value + '0'; - txbuff[2] = ' '; - txbuff[3] = (uint8_t)ResponseMsgParamCodes::Accepted; - txbuff[4] = findaValue + '0'; - txbuff[5] = '\n'; - return 6; - } - - uint8_t Protocol::EncodeResponseVersion(const RequestMsg &msg, uint8_t value, uint8_t *txbuff) { - txbuff[0] = (uint8_t)msg.code; - txbuff[1] = msg.value + '0'; - txbuff[2] = ' '; - txbuff[3] = (uint8_t)ResponseMsgParamCodes::Accepted; - uint8_t *dst = txbuff + 4; +uint8_t Protocol::EncodeResponseQueryOperation(const RequestMsg &msg, ResponseMsgParamCodes code, uint8_t value, uint8_t *txbuff) { + txbuff[0] = (uint8_t)msg.code; + txbuff[1] = msg.value + '0'; + txbuff[2] = ' '; + txbuff[3] = (uint8_t)code; + uint8_t *dst = txbuff + 4; + if (code != ResponseMsgParamCodes::Finished) { if (value < 10) { *dst++ = value + '0'; } else if (value < 100) { @@ -193,31 +214,10 @@ namespace protocol { *dst++ = (value / 10) % 10 + '0'; *dst++ = value % 10 + '0'; } - *dst = '\n'; - return dst - txbuff + 1; - } - - uint8_t Protocol::EncodeResponseQueryOperation(const RequestMsg &msg, ResponseMsgParamCodes code, uint8_t value, uint8_t *txbuff) { - txbuff[0] = (uint8_t)msg.code; - txbuff[1] = msg.value + '0'; - txbuff[2] = ' '; - txbuff[3] = (uint8_t)code; - uint8_t *dst = txbuff + 4; - if (code != ResponseMsgParamCodes::Finished) { - if (value < 10) { - *dst++ = value + '0'; - } else if (value < 100) { - *dst++ = value / 10 + '0'; - *dst++ = value % 10 + '0'; - } else { - *dst++ = value / 100 + '0'; - *dst++ = (value / 10) % 10 + '0'; - *dst++ = value % 10 + '0'; - } - } - *dst = '\n'; - return dst - txbuff + 1; } + *dst = '\n'; + return dst - txbuff + 1; +} } // namespace protocol } // namespace modules diff --git a/src/modules/protocol.h b/src/modules/protocol.h index 98a8d61..a9ee3c2 100644 --- a/src/modules/protocol.h +++ b/src/modules/protocol.h @@ -9,137 +9,137 @@ namespace modules { namespace protocol { - enum class RequestMsgCodes : uint8_t { - unknown = 0, - Query = 'Q', - Tool = 'T', - Load = 'L', - Mode = 'M', - Unload = 'U', - Reset = 'X', - Finda = 'P', - Version = 'S', - Button = 'B', - Eject = 'E', - Wait = 'W', - Cut = 'K' +enum class RequestMsgCodes : uint8_t { + unknown = 0, + Query = 'Q', + Tool = 'T', + Load = 'L', + Mode = 'M', + Unload = 'U', + Reset = 'X', + Finda = 'P', + Version = 'S', + Button = 'B', + Eject = 'E', + Wait = 'W', + Cut = 'K' +}; + +enum class ResponseMsgParamCodes : uint8_t { + unknown = 0, + Processing = 'P', + Error = 'E', + Finished = 'F', + Accepted = 'A', + Rejected = 'R' +}; + +/// A request message +/// Requests are being sent by the printer into the MMU +/// It is the same structure as the generic Msg +struct RequestMsg { + RequestMsgCodes code; + uint8_t value; + inline RequestMsg(RequestMsgCodes code, uint8_t value) + : code(code) + , value(value) {} +}; + +/// A response message +/// Responses are being sent from the MMU into the printer as a response to a request message +struct ResponseMsg { + RequestMsg request; ///< response is always preceeded by the request message + ResponseMsgParamCodes paramCode; ///< parameters of reply + uint8_t paramValue; ///< parameters of reply + inline ResponseMsg(RequestMsg request, ResponseMsgParamCodes paramCode, uint8_t paramValue) + : request(request) + , paramCode(paramCode) + , paramValue(paramValue) {} +}; + +/// Message decoding return value +enum class DecodeStatus : uint_fast8_t { + MessageCompleted, ///< message completed and successfully lexed + NeedMoreData, ///< message incomplete yet, waiting for another byte to come + Error, ///< input character broke message decoding +}; + +/// Protocol class is responsible for creating/decoding messages in Rx/Tx buffer +/// Beware - in the decoding more, it is meant to be a statefull instance which works through public methods +/// processing one input byte per call +class Protocol { +public: + inline Protocol() + : rqState(RequestStates::Code) + , requestMsg(RequestMsgCodes::unknown, 0) + , rspState(ResponseStates::RequestCode) + , responseMsg(RequestMsg(RequestMsgCodes::unknown, 0), ResponseMsgParamCodes::unknown, 0) { + } + + /// Takes the input byte c and steps one step through the state machine + /// @returns state of the message being decoded + DecodeStatus DecodeRequest(uint8_t c); + + /// Decodes response message in rxbuff + /// @returns decoded response message structure + DecodeStatus DecodeResponse(uint8_t c); + + /// Encodes request message msg into txbuff memory + /// It is expected the txbuff is large enough to fit the message + /// @returns number of bytes written into txbuff + static uint8_t EncodeRequest(const RequestMsg &msg, uint8_t *txbuff); + + /// Encode generic response Command Accepted or Rejected + /// @param msg source request message for this response + /// @returns number of bytes written into txbuff + static uint8_t EncodeResponseCmdAR(const RequestMsg &msg, ResponseMsgParamCodes ar, uint8_t *txbuff); + + /// Encode response to Read FINDA query + /// @param msg source request message for this response + /// @param findaValue 1/0 (on/off) status of FINDA + /// @returns number of bytes written into txbuff + static uint8_t EncodeResponseReadFINDA(const RequestMsg &msg, uint8_t findaValue, uint8_t *txbuff); + + /// Encode response to Version query + /// @param msg source request message for this response + /// @param value version number (0-255) + /// @returns number of bytes written into txbuff + static uint8_t EncodeResponseVersion(const RequestMsg &msg, uint8_t value, uint8_t *txbuff); + + /// Encode response to Query operation status + /// @param msg source request message for this response + /// @param code status of operation (Processing, Error, Finished) + /// @param value related to status of operation(e.g. error code or progress) + /// @returns number of bytes written into txbuff + static uint8_t EncodeResponseQueryOperation(const RequestMsg &msg, ResponseMsgParamCodes code, uint8_t value, uint8_t *txbuff); + + /// @returns the most recently lexed request message + inline const RequestMsg GetRequestMsg() const { return requestMsg; } + + /// @returns the most recently lexed response message + inline const ResponseMsg GetResponseMsg() const { return responseMsg; } + +private: + enum class RequestStates : uint8_t { + Code, ///< starting state - expects message code + Value, ///< expecting code value + Error ///< automaton in error state }; - enum class ResponseMsgParamCodes : uint8_t { - unknown = 0, - Processing = 'P', - Error = 'E', - Finished = 'F', - Accepted = 'A', - Rejected = 'R' + RequestStates rqState; + RequestMsg requestMsg; + + enum class ResponseStates : uint8_t { + RequestCode, ///< starting state - expects message code + RequestValue, ///< expecting code value + ParamCode, ///< expecting param code + ParamValue, ///< expecting param value + Error ///< automaton in error state }; - /// A request message - /// Requests are being sent by the printer into the MMU - /// It is the same structure as the generic Msg - struct RequestMsg { - RequestMsgCodes code; - uint8_t value; - inline RequestMsg(RequestMsgCodes code, uint8_t value) - : code(code) - , value(value) {} - }; - - /// A response message - /// Responses are being sent from the MMU into the printer as a response to a request message - struct ResponseMsg { - RequestMsg request; ///< response is always preceeded by the request message - ResponseMsgParamCodes paramCode; ///< parameters of reply - uint8_t paramValue; ///< parameters of reply - inline ResponseMsg(RequestMsg request, ResponseMsgParamCodes paramCode, uint8_t paramValue) - : request(request) - , paramCode(paramCode) - , paramValue(paramValue) {} - }; - - /// Message decoding return value - enum class DecodeStatus : uint_fast8_t { - MessageCompleted, ///< message completed and successfully lexed - NeedMoreData, ///< message incomplete yet, waiting for another byte to come - Error, ///< input character broke message decoding - }; - - /// Protocol class is responsible for creating/decoding messages in Rx/Tx buffer - /// Beware - in the decoding more, it is meant to be a statefull instance which works through public methods - /// processing one input byte per call - class Protocol { - public: - inline Protocol() - : rqState(RequestStates::Code) - , requestMsg(RequestMsgCodes::unknown, 0) - , rspState(ResponseStates::RequestCode) - , responseMsg(RequestMsg(RequestMsgCodes::unknown, 0), ResponseMsgParamCodes::unknown, 0) { - } - - /// Takes the input byte c and steps one step through the state machine - /// @returns state of the message being decoded - DecodeStatus DecodeRequest(uint8_t c); - - /// Decodes response message in rxbuff - /// @returns decoded response message structure - DecodeStatus DecodeResponse(uint8_t c); - - /// Encodes request message msg into txbuff memory - /// It is expected the txbuff is large enough to fit the message - /// @returns number of bytes written into txbuff - static uint8_t EncodeRequest(const RequestMsg &msg, uint8_t *txbuff); - - /// Encode generic response Command Accepted or Rejected - /// @param msg source request message for this response - /// @returns number of bytes written into txbuff - static uint8_t EncodeResponseCmdAR(const RequestMsg &msg, ResponseMsgParamCodes ar, uint8_t *txbuff); - - /// Encode response to Read FINDA query - /// @param msg source request message for this response - /// @param findaValue 1/0 (on/off) status of FINDA - /// @returns number of bytes written into txbuff - static uint8_t EncodeResponseReadFINDA(const RequestMsg &msg, uint8_t findaValue, uint8_t *txbuff); - - /// Encode response to Version query - /// @param msg source request message for this response - /// @param value version number (0-255) - /// @returns number of bytes written into txbuff - static uint8_t EncodeResponseVersion(const RequestMsg &msg, uint8_t value, uint8_t *txbuff); - - /// Encode response to Query operation status - /// @param msg source request message for this response - /// @param code status of operation (Processing, Error, Finished) - /// @param value related to status of operation(e.g. error code or progress) - /// @returns number of bytes written into txbuff - static uint8_t EncodeResponseQueryOperation(const RequestMsg &msg, ResponseMsgParamCodes code, uint8_t value, uint8_t *txbuff); - - /// @returns the most recently lexed request message - inline const RequestMsg GetRequestMsg() const { return requestMsg; } - - /// @returns the most recently lexed response message - inline const ResponseMsg GetResponseMsg() const { return responseMsg; } - - private: - enum class RequestStates : uint8_t { - Code, ///< starting state - expects message code - Value, ///< expecting code value - Error ///< automaton in error state - }; - - RequestStates rqState; - RequestMsg requestMsg; - - enum class ResponseStates : uint8_t { - RequestCode, ///< starting state - expects message code - RequestValue, ///< expecting code value - ParamCode, ///< expecting param code - ParamValue, ///< expecting param value - Error ///< automaton in error state - }; - - ResponseStates rspState; - ResponseMsg responseMsg; - }; + ResponseStates rspState; + ResponseMsg responseMsg; +}; } // namespace protocol } // namespace modules diff --git a/tests/unit/modules/buttons/stub_adc.cpp b/tests/unit/modules/buttons/stub_adc.cpp index 5006f68..5cf7593 100644 --- a/tests/unit/modules/buttons/stub_adc.cpp +++ b/tests/unit/modules/buttons/stub_adc.cpp @@ -5,28 +5,28 @@ namespace hal { namespace adc { - static TADCData values2Return; - static TADCData::const_iterator rdptr = values2Return.cbegin(); - static uint8_t oversampleFactor = 1; - static uint8_t oversample = 1; ///< current count of oversampled values returned from the ADC - will get filled with oversampleFactor once it reaches zero +static TADCData values2Return; +static TADCData::const_iterator rdptr = values2Return.cbegin(); +static uint8_t oversampleFactor = 1; +static uint8_t oversample = 1; ///< current count of oversampled values returned from the ADC - will get filled with oversampleFactor once it reaches zero - void ReinitADC(TADCData &&d, uint8_t ovsmpl) { - values2Return = std::move(d); - oversampleFactor = ovsmpl; - oversample = ovsmpl; - rdptr = values2Return.cbegin(); - } +void ReinitADC(TADCData &&d, uint8_t ovsmpl) { + values2Return = std::move(d); + oversampleFactor = ovsmpl; + oversample = ovsmpl; + rdptr = values2Return.cbegin(); +} - /// ADC access routines - uint16_t ReadADC(uint8_t /*adc*/) { - if (!oversample) { - ++rdptr; - oversample = oversampleFactor; - } else { - --oversample; - } - return rdptr != values2Return.end() ? *rdptr : 1023; +/// ADC access routines +uint16_t ReadADC(uint8_t /*adc*/) { + if (!oversample) { + ++rdptr; + oversample = oversampleFactor; + } else { + --oversample; } + return rdptr != values2Return.end() ? *rdptr : 1023; +} } // namespace adc } // namespace hal diff --git a/tests/unit/modules/buttons/stub_adc.h b/tests/unit/modules/buttons/stub_adc.h index ae3b658..4f76f70 100644 --- a/tests/unit/modules/buttons/stub_adc.h +++ b/tests/unit/modules/buttons/stub_adc.h @@ -6,9 +6,9 @@ namespace hal { namespace adc { - using TADCData = std::vector; +using TADCData = std::vector; - void ReinitADC(TADCData &&d, uint8_t ovsmpl); +void ReinitADC(TADCData &&d, uint8_t ovsmpl); } // namespace adc } // namespace hal From c377674aeeaee308c929fd133407b06218a65ef2 Mon Sep 17 00:00:00 2001 From: "D.R.racer" Date: Tue, 25 May 2021 12:33:50 +0200 Subject: [PATCH 5/7] Prepare unit tests for LEDs --- tests/unit/modules/CMakeLists.txt | 1 + tests/unit/modules/leds/CMakeLists.txt | 10 ++++++++++ tests/unit/modules/leds/test_leds.cpp | 8 ++++++++ 3 files changed, 19 insertions(+) create mode 100644 tests/unit/modules/leds/CMakeLists.txt create mode 100644 tests/unit/modules/leds/test_leds.cpp diff --git a/tests/unit/modules/CMakeLists.txt b/tests/unit/modules/CMakeLists.txt index 4203bf3..442fa8a 100644 --- a/tests/unit/modules/CMakeLists.txt +++ b/tests/unit/modules/CMakeLists.txt @@ -1,2 +1,3 @@ add_subdirectory(buttons) +add_subdirectory(leds) add_subdirectory(protocol) diff --git a/tests/unit/modules/leds/CMakeLists.txt b/tests/unit/modules/leds/CMakeLists.txt new file mode 100644 index 0000000..9f39584 --- /dev/null +++ b/tests/unit/modules/leds/CMakeLists.txt @@ -0,0 +1,10 @@ +# define the test executable +add_executable(leds_tests ../../../../src/modules/leds.cpp test_leds.cpp) + +# define required search paths +target_include_directories( + leds_tests PUBLIC ${CMAKE_SOURCE_DIR}/src/modules ${CMAKE_SOURCE_DIR}/src/hal + ) + +# tell build system about the test case +add_catch_test(leds_tests) diff --git a/tests/unit/modules/leds/test_leds.cpp b/tests/unit/modules/leds/test_leds.cpp new file mode 100644 index 0000000..68a9cff --- /dev/null +++ b/tests/unit/modules/leds/test_leds.cpp @@ -0,0 +1,8 @@ +#include "catch2/catch.hpp" +#include "leds.h" + +using Catch::Matchers::Equals; + +TEST_CASE("leds::basic", "[leds]") { + using namespace modules::leds; +} From 004db59da0c01af776c21fdece023c4056a7b969 Mon Sep 17 00:00:00 2001 From: "D.R.racer" Date: Wed, 26 May 2021 09:29:32 +0200 Subject: [PATCH 6/7] Add shift register interface and improve LED integration with it --- CMakeLists.txt | 1 + src/hal/avr/shr16.cpp | 57 +++++++++++++++++++++++++++++++++++++++++++ src/hal/shr16.h | 55 +++++++++++++++++++++++++++++++++++++++++ src/main.cpp | 49 +++++++++++++++++++------------------ src/modules/leds.cpp | 5 ++-- src/modules/leds.h | 3 +-- 6 files changed, 142 insertions(+), 28 deletions(-) create mode 100644 src/hal/avr/shr16.cpp create mode 100644 src/hal/shr16.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 16f8f69..8faa2ee 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -187,6 +187,7 @@ target_sources( PRIVATE src/main.cpp src/hal/avr/cpu.cpp src/hal/avr/usart.cpp + src/hal/avr/shr16.cpp src/hal/adc.cpp src/modules/protocol.cpp src/modules/buttons.cpp diff --git a/src/hal/avr/shr16.cpp b/src/hal/avr/shr16.cpp new file mode 100644 index 0000000..50cda5e --- /dev/null +++ b/src/hal/avr/shr16.cpp @@ -0,0 +1,57 @@ +#include "../shr16.h" +#include "../gpio.h" + +namespace hal { +namespace shr16 { + +SHR16 shr16; + +void SHR16::Init() { + // DDRC |= 0x80; + // DDRB |= 0x40; + // DDRB |= 0x20; + // PORTC &= ~0x80; + // PORTB &= ~0x40; + // PORTB &= ~0x20; + // shr16_v = 0; + // Write(shr16_v); + // Write(shr16_v); +} + +void SHR16::Write(uint16_t v) { + // PORTB &= ~0x40; + // asm("nop"); + // for (uint16_t m = 0x8000; m; m >>= 1) + // { + // if (m & v) + // PORTB |= 0x20; + // else + // PORTB &= ~0x20; + // PORTC |= 0x80; + // asm("nop"); + // PORTC &= ~0x80; + // asm("nop"); + // } + // PORTB |= 0x40; + // asm("nop"); + // shr16_v = v; +} + +void SHR16::SetLED(uint16_t led) { + // led = ((led & 0x00ff) << 8) | ((led & 0x0300) >> 2); + // Write((shr16_v & ~SHR16_LED_MSK) | led); +} + +void SHR16::SetTMCEnabled(uint8_t ena) { + // ena ^= 7; + // ena = ((ena & 1) << 1) | ((ena & 2) << 2) | ((ena & 4) << 3); // 0. << 1 == 1., 1. << 2 == 3., 2. << 3 == 5. + // Write((shr16_v & ~SHR16_ENA_MSK) | ena); +} + +void SHR16::SetTMCDir(uint8_t dir) { + // dir = (dir & 1) | ((dir & 2) << 1) | ((dir & 4) << 2); // 0., 1. << 1 == 2., 2. << 2 == 4. + // Write((shr16_v & ~SHR16_DIR_MSK) | dir); +} + +} // namespace shr16 +} // namespace hal diff --git a/src/hal/shr16.h b/src/hal/shr16.h new file mode 100644 index 0000000..7120696 --- /dev/null +++ b/src/hal/shr16.h @@ -0,0 +1,55 @@ +#pragma once + +#include + +namespace hal { +namespace shr16 { + +/// 16bit shift register (2x74595) interface +/// +/// The pinout is hard coded as follows: +/// SHR16_CLK: signal d13 - PC7 +/// SHR16_LAT: signal d10 - PB6 +/// SHR16_DAT: signal d9 - PB5 +/// +/// Shift register outputs: +/// LEDS - hardcoded +/// SHR16_LEDG0 = 0x0100 +/// SHR16_LEDR0 = 0x0200 +/// SHR16_LEDG1 = 0x0400 +/// SHR16_LEDR1 = 0x0800 +/// SHR16_LEDG2 = 0x1000 +/// SHR16_LEDR2 = 0x2000 +/// SHR16_LEDG3 = 0x4000 +/// SHR16_LEDR3 = 0x8000 +/// SHR16_LEDG4 = 0x0040 +/// SHR16_LEDR4 = 0x0080 +/// SHR16_LED_MSK = 0xffc0 +/// +/// TMC2130 Direction/Enable signals - hardcoded +/// SHR16_DIR_0 = 0x0001 +/// SHR16_ENA_0 = 0x0002 +/// SHR16_DIR_1 = 0x0004 +/// SHR16_ENA_1 = 0x0008 +/// SHR16_DIR_2 = 0x0010 +/// SHR16_ENA_2 = 0x0020 +/// +/// SHR16_DIR_MSK = (SHR16_DIR_0 + SHR16_DIR_1 + SHR16_DIR_2) +/// SHR16_ENA_MSK = (SHR16_ENA_0 + SHR16_ENA_1 + SHR16_ENA_2) +class SHR16 { + +public: + void Init(); + void SetLED(uint16_t led); + void SetTMCEnabled(uint8_t ena); + void SetTMCDir(uint8_t dir); + +private: + uint16_t shr16_v; + void Write(uint16_t v); +}; + +extern SHR16 shr16; + +} // namespace shr16 +} // namespace hal diff --git a/src/main.cpp b/src/main.cpp index 0667feb..4cb639a 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -3,6 +3,7 @@ #include "hal/gpio.h" #include "hal/spi.h" #include "hal/usart.h" +#include "hal/shr16.h" #include "pins.h" #include @@ -22,23 +23,23 @@ void TmpPlayground() { using namespace hal; // SPI example - gpio::Init(gpio::GPIO_pin(GPIOC, 6), gpio::GPIO_InitTypeDef(gpio::Mode::output, gpio::Level::high)); - uint8_t dat[5]; - gpio::WritePin(gpio::GPIO_pin(GPIOC, 6), gpio::Level::low); - spi::TxRx(SPI0, 0x01); - spi::TxRx(SPI0, 0x00); - spi::TxRx(SPI0, 0x00); - spi::TxRx(SPI0, 0x00); - spi::TxRx(SPI0, 0x00); - gpio::WritePin(gpio::GPIO_pin(GPIOC, 6), gpio::Level::high); - gpio::WritePin(gpio::GPIO_pin(GPIOC, 6), gpio::Level::low); - dat[0] = spi::TxRx(SPI0, 0x00); - dat[1] = spi::TxRx(SPI0, 0x00); - dat[2] = spi::TxRx(SPI0, 0x00); - dat[3] = spi::TxRx(SPI0, 0x00); - dat[4] = spi::TxRx(SPI0, 0x00); - gpio::WritePin(gpio::GPIO_pin(GPIOC, 6), gpio::Level::high); - (void)dat; + // gpio::Init(gpio::GPIO_pin(GPIOC, 6), gpio::GPIO_InitTypeDef(gpio::Mode::output, gpio::Level::high)); + // uint8_t dat[5]; + // gpio::WritePin(gpio::GPIO_pin(GPIOC, 6), gpio::Level::low); + // spi::TxRx(SPI0, 0x01); + // spi::TxRx(SPI0, 0x00); + // spi::TxRx(SPI0, 0x00); + // spi::TxRx(SPI0, 0x00); + // spi::TxRx(SPI0, 0x00); + // gpio::WritePin(gpio::GPIO_pin(GPIOC, 6), gpio::Level::high); + // gpio::WritePin(gpio::GPIO_pin(GPIOC, 6), gpio::Level::low); + // dat[0] = spi::TxRx(SPI0, 0x00); + // dat[1] = spi::TxRx(SPI0, 0x00); + // dat[2] = spi::TxRx(SPI0, 0x00); + // dat[3] = spi::TxRx(SPI0, 0x00); + // dat[4] = spi::TxRx(SPI0, 0x00); + // gpio::WritePin(gpio::GPIO_pin(GPIOC, 6), gpio::Level::high); + // (void)dat; // using namespace hal::gpio; // WritePin(GPIO_pin(GPIOB, 5), Level::low); @@ -68,9 +69,9 @@ void setup() { cpu::Init(); - // shr::Init() + shr16::shr16.Init(); leds.SetMode(4, false, modules::leds::Mode::blink0); - // shr::Send(leds.Step(0)); + leds.Step(0); // @@TODO if the shift register doesn't work we really can't signalize anything, only internal variables will be accessible if the UART works @@ -81,7 +82,7 @@ void setup() { }; hal::usart::usart1.Init(&usart_conf); leds.SetMode(3, false, modules::leds::Mode::on); - // shr::Send(leds.Step(0)); + leds.Step(0); // @@TODO if both shift register and the UART are dead, we are sitting ducks :( @@ -96,15 +97,15 @@ void setup() { }; spi::Init(SPI0, &spi_conf); leds.SetMode(2, false, modules::leds::Mode::on); - //shr::Send(leds.Step(0)); + leds.Step(0); // tmc::Init() leds.SetMode(1, false, modules::leds::Mode::on); - //shr::Send(leds.Step(0)); + leds.Step(0); // adc::Init(); leds.SetMode(0, false, modules::leds::Mode::on); - //shr::Send(leds.Step(0)); + leds.Step(0); } void ProcessRequestMsg(const modules::protocol::RequestMsg &rq) { @@ -149,7 +150,7 @@ void loop() { ProcessRequestMsg(protocol.GetRequestMsg()); } buttons.Step(hal::adc::ReadADC(0)); - // shr.Send(leds.Step(0)); + leds.Step(0); } int main() { diff --git a/src/modules/leds.cpp b/src/modules/leds.cpp index 4787e92..631b64f 100644 --- a/src/modules/leds.cpp +++ b/src/modules/leds.cpp @@ -1,4 +1,5 @@ #include "leds.h" +#include "hal/shr16.h" namespace modules { namespace leds { @@ -35,7 +36,7 @@ bool LED::Step(bool oddPeriod) { } } -uint16_t LEDs::Step(uint8_t delta_ms) { +void LEDs::Step(uint8_t delta_ms) { ms += delta_ms; bool oddPeriod = ((ms / 1000U) & 0x01U) != 0; uint16_t result = 0; @@ -43,7 +44,7 @@ uint16_t LEDs::Step(uint8_t delta_ms) { result <<= 1; result |= leds[i].Step(oddPeriod); } - return result; + hal::shr16::shr16.SetLED(result); } } // namespace leds diff --git a/src/modules/leds.h b/src/modules/leds.h index 057113e..801139a 100644 --- a/src/modules/leds.h +++ b/src/modules/leds.h @@ -51,8 +51,7 @@ public: : ms(0) {}; /// step LED automaton - /// @returns statuses of LEDs - one bit per LED and 1 = on, 0 = off - uint16_t Step(uint8_t delta_ms); + void Step(uint8_t delta_ms); inline constexpr uint8_t LedPairsCount() const { return ledPairs; } From bd335ec30d99c276f386a3e60c88210b54c22eb4 Mon Sep 17 00:00:00 2001 From: "D.R.racer" Date: Wed, 26 May 2021 11:39:24 +0200 Subject: [PATCH 7/7] Unit test for LEDs interface + fixes --- src/modules/leds.cpp | 8 +- src/modules/leds.h | 26 ++++-- tests/unit/modules/leds/CMakeLists.txt | 2 +- tests/unit/modules/leds/stub_shr16.cpp | 28 +++++++ tests/unit/modules/leds/test_leds.cpp | 107 ++++++++++++++++++++++++- 5 files changed, 161 insertions(+), 10 deletions(-) create mode 100644 tests/unit/modules/leds/stub_shr16.cpp diff --git a/src/modules/leds.cpp b/src/modules/leds.cpp index 631b64f..23fba84 100644 --- a/src/modules/leds.cpp +++ b/src/modules/leds.cpp @@ -1,5 +1,5 @@ #include "leds.h" -#include "hal/shr16.h" +#include "../hal/shr16.h" namespace modules { namespace leds { @@ -34,16 +34,18 @@ bool LED::Step(bool oddPeriod) { default: // do nothing break; } + return state.on; } -void LEDs::Step(uint8_t delta_ms) { +void LEDs::Step(uint16_t delta_ms) { ms += delta_ms; bool oddPeriod = ((ms / 1000U) & 0x01U) != 0; uint16_t result = 0; - for (uint8_t i = 0; i < ledPairs * 2; ++i) { + for (int8_t i = ledPairs * 2 - 1; i >= 0; --i) { result <<= 1; result |= leds[i].Step(oddPeriod); } + hal::shr16::shr16.SetLED(result); } diff --git a/src/modules/leds.h b/src/modules/leds.h index 801139a..cee5062 100644 --- a/src/modules/leds.h +++ b/src/modules/leds.h @@ -22,6 +22,11 @@ enum Mode { blink1 ///< start blinking at odd periods }; +enum Color { + green = 0, + red = 1 +}; + /// a single LED class LED { public: @@ -51,12 +56,12 @@ public: : ms(0) {}; /// step LED automaton - void Step(uint8_t delta_ms); + void Step(uint16_t delta_ms); inline constexpr uint8_t LedPairsCount() const { return ledPairs; } - inline void SetMode(uint8_t slot, bool red, Mode mode) { - SetMode(slot * 2 + red, mode); + inline void SetMode(uint8_t slot, Color color, Mode mode) { + SetMode(slot * 2 + color, mode); } inline void SetMode(uint8_t index, Mode mode) { leds[index].SetMode(mode); @@ -65,12 +70,23 @@ public: inline bool LedOn(uint8_t index) const { return leds[index].On(); } - inline bool LedOn(uint8_t slot, bool red) const { - return leds[slot * 2 + red].On(); + inline bool LedOn(uint8_t slot, Color color) const { + return leds[slot * 2 + color].On(); } private: constexpr static const uint8_t ledPairs = 5; + /// pairs of LEDs: + /// [0] - green LED slot 0 + /// [1] - red LED slot 0 + /// [2] - green LED slot 1 + /// [3] - red LED slot 1 + /// [4] - green LED slot 2 + /// [5] - red LED slot 2 + /// [6] - green LED slot 3 + /// [7] - red LED slot 3 + /// [8] - green LED slot 4 + /// [9] - red LED slot 4 LED leds[ledPairs * 2]; uint16_t ms; }; diff --git a/tests/unit/modules/leds/CMakeLists.txt b/tests/unit/modules/leds/CMakeLists.txt index 9f39584..7c8a23e 100644 --- a/tests/unit/modules/leds/CMakeLists.txt +++ b/tests/unit/modules/leds/CMakeLists.txt @@ -1,5 +1,5 @@ # define the test executable -add_executable(leds_tests ../../../../src/modules/leds.cpp test_leds.cpp) +add_executable(leds_tests ../../../../src/modules/leds.cpp stub_shr16.cpp test_leds.cpp) # define required search paths target_include_directories( diff --git a/tests/unit/modules/leds/stub_shr16.cpp b/tests/unit/modules/leds/stub_shr16.cpp new file mode 100644 index 0000000..6433ad4 --- /dev/null +++ b/tests/unit/modules/leds/stub_shr16.cpp @@ -0,0 +1,28 @@ +#include "shr16.h" + +namespace hal { +namespace shr16 { + +SHR16 shr16; + +uint16_t shr16_v_copy; + +void SHR16::Init() { + shr16_v_copy = 0; +} + +void SHR16::SetLED(uint16_t led) { + shr16_v_copy = ((led & 0x00ff) << 8) | ((led & 0x0300) >> 2); +} +void SHR16::SetTMCEnabled(uint8_t ena) { + // do nothing right now +} +void SHR16::SetTMCDir(uint8_t dir) { + // do nothing right now +} +void SHR16::Write(uint16_t v) { + // do nothing right now +} + +} // namespace shr16 +} // namespace hal diff --git a/tests/unit/modules/leds/test_leds.cpp b/tests/unit/modules/leds/test_leds.cpp index 68a9cff..0a6ca3b 100644 --- a/tests/unit/modules/leds/test_leds.cpp +++ b/tests/unit/modules/leds/test_leds.cpp @@ -1,8 +1,113 @@ #include "catch2/catch.hpp" #include "leds.h" +#include "shr16.h" using Catch::Matchers::Equals; -TEST_CASE("leds::basic", "[leds]") { +namespace hal { +namespace shr16 { +extern uint16_t shr16_v_copy; +} // namespace shr16 +} // namespace hal + +/// LEDS - hardcoded +#define SHR16_LEDG0 0x0100 +#define SHR16_LEDR0 0x0200 +#define SHR16_LEDG1 0x0400 +#define SHR16_LEDR1 0x0800 +#define SHR16_LEDG2 0x1000 +#define SHR16_LEDR2 0x2000 +#define SHR16_LEDG3 0x4000 +#define SHR16_LEDR3 0x8000 +#define SHR16_LEDG4 0x0040 +#define SHR16_LEDR4 0x0080 + +TEST_CASE("leds::single", "[leds]") { using namespace modules::leds; + using namespace hal::shr16; + + LEDs leds; + + uint8_t index; + Color color; + uint16_t shr16_register; + std::tie(index, color, shr16_register) = GENERATE( + std::make_tuple(0, green, SHR16_LEDG0), + std::make_tuple(0, red, SHR16_LEDR0), + std::make_tuple(1, green, SHR16_LEDG1), + std::make_tuple(1, red, SHR16_LEDR1), + std::make_tuple(2, green, SHR16_LEDG2), + std::make_tuple(2, red, SHR16_LEDR2), + std::make_tuple(3, green, SHR16_LEDG3), + std::make_tuple(3, red, SHR16_LEDR3), + std::make_tuple(4, green, SHR16_LEDG4), + std::make_tuple(4, red, SHR16_LEDR4)); + + shr16.Init(); // clears the register for the test + + // turn LED on + leds.SetMode(index, color, on); + leds.Step(0); + CHECK(leds.LedOn(index, color) == true); + CHECK(shr16_v_copy == shr16_register); + + // turn LED off + leds.SetMode(index, color, off); + leds.Step(0); + CHECK(leds.LedOn(index, color) == false); + CHECK(shr16_v_copy == 0); +} + +void TestBlink(uint8_t index, modules::leds::Color color, uint16_t shr16_register, bool shouldBeOn, modules::leds::Mode blinkMode) { + using namespace modules::leds; + using namespace hal::shr16; + LEDs leds; + + leds.SetMode(index, color, blinkMode); + leds.Step(1); + + REQUIRE(leds.LedOn(index, color) == shouldBeOn); + CHECK(shr16_v_copy == (shouldBeOn ? shr16_register : 0)); + + // test 4 seconds of blinking + for (uint8_t s = 1; s < 4; ++s) { + // one second elapsed ;) + leds.Step(1000); + shouldBeOn = !shouldBeOn; + CHECK(leds.LedOn(index, color) == shouldBeOn); + CHECK(shr16_v_copy == (shouldBeOn ? shr16_register : 0)); + } + + // turn LED off + leds.SetMode(index, color, off); + leds.Step(0); + CHECK(leds.LedOn(index, color) == false); + CHECK(shr16_v_copy == 0); +} + +TEST_CASE("leds::blink0-single", "[leds]") { + using namespace modules::leds; + using namespace hal::shr16; + + uint8_t index; + Color color; + uint16_t shr16_register; + std::tie(index, color, shr16_register) = GENERATE( + std::make_tuple(0, green, SHR16_LEDG0), + std::make_tuple(0, red, SHR16_LEDR0), + std::make_tuple(1, green, SHR16_LEDG1), + std::make_tuple(1, red, SHR16_LEDR1), + std::make_tuple(2, green, SHR16_LEDG2), + std::make_tuple(2, red, SHR16_LEDR2), + std::make_tuple(3, green, SHR16_LEDG3), + std::make_tuple(3, red, SHR16_LEDR3), + std::make_tuple(4, green, SHR16_LEDG4), + std::make_tuple(4, red, SHR16_LEDR4)); + + shr16.Init(); // clears the register for the test + + // set LED into blink0 mode - on in even periods of 1s intervals, starts as OFF + TestBlink(index, color, shr16_register, false, blink0); + + TestBlink(index, color, shr16_register, true, blink1); }