Merge pull request #13 from DRracer/leds

Add interface module for driving LEDs

.clang-format

@@ -84,7 +84,7 @@ KeepEmptyLinesAtTheStartOfBlocks: true
 MacroBlockBegin: ''
 MacroBlockEnd:   ''
 MaxEmptyLinesToKeep: 1
-NamespaceIndentation: Inner
+NamespaceIndentation: None
 ObjCBinPackProtocolList: Auto
 ObjCBlockIndentWidth: 4
 ObjCSpaceAfterProperty: true

CMakeLists.txt

@@ -183,8 +183,15 @@ 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
+  firmware
+  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
+          src/modules/leds.cpp
   )
 
 set_property(

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

src/hal/adc.h

@@ -4,10 +4,10 @@
 /// Hardware Abstraction Layer for the ADC's
 
 namespace hal {
-namespace ADC {
+namespace adc {
 
-    /// ADC access routines
+/// ADC access routines
-    uint16_t ReadADC(uint8_t adc);
+uint16_t ReadADC(uint8_t adc);
 
-} // namespace ADC
+} // namespace adc
 } // namespace hal

src/hal/avr/cpu.cpp

@@ -1,10 +1,10 @@
 #include "../cpu.h"
 
 namespace hal {
-namespace CPU {
+namespace cpu {
 
-    void Init() {
+void Init() {
-    }
+}
 
 } // namespace CPU
 } // namespace hal

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

src/hal/avr/usart.cpp

@@ -1,13 +1,18 @@
 #include "../usart.h"
 #include <avr/interrupt.h>
 
-uint8_t hal::USART::Read() {
+namespace hal {
+namespace usart {
+
+USART usart1;
+
+uint8_t USART::Read() {
     uint8_t c = 0;
-    this->rx_buf.ConsumeFirst(c);
+    rx_buf.ConsumeFirst(c);
     return c;
 }
 
-void hal::USART::Write(uint8_t 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
@@ -27,8 +32,9 @@ void hal::USART::Write(uint8_t c) {
             // 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))
+            if (husart->UCSRxA & (1 << 5)) {
                 ISR_UDRE();
+            }
         } else {
             // nop, the interrupt handler will free up space for us
         }
@@ -37,36 +43,40 @@ void hal::USART::Write(uint8_t c) {
     husart->UCSRxB |= (1 << 5); //enable UDRE interrupt
 }
 
-void hal::USART::Flush() {
+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)
+    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))
+            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) {
+void USART::puts(const char *str) {
-    while (*str)
+    while (*str) {
-        this->Write(*str++);
+        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();
 }

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)
+/// CPU init routines (not really necessary for the AVR)
-    void Init();
+void Init();
 
-} // namespace CPU
+} // namespace cpu
 } // namespace hal

src/hal/eeprom.h

@@ -3,10 +3,10 @@
 namespace hal {
 namespace EEPROM {
 
-    /// EEPROM interface
+/// EEPROM interface
-    void WriteByte(uint16_t addr, uint8_t value);
+void WriteByte(uint16_t addr, uint8_t value);
-    void UpdateByte(uint16_t addr, uint8_t value);
+void UpdateByte(uint16_t addr, uint8_t value);
-    uint8_t ReadByte(uint16_t addr);
+uint8_t ReadByte(uint16_t addr);
 
 } // namespace EEPROM
 } // namespace hal

src/hal/gpio.h

@@ -5,29 +5,29 @@
 namespace hal {
 namespace gpio {
 
-    struct GPIO_TypeDef {
+struct GPIO_TypeDef {
     volatile uint8_t PINx;
     volatile uint8_t DDRx;
     volatile uint8_t PORTx;
-    };
+};
 
-    enum class Mode : uint8_t {
+enum class Mode : uint8_t {
     input = 0,
     output,
-    };
+};
 
-    enum class Pull : uint8_t {
+enum class Pull : uint8_t {
     none = 0,
     up,
     down, //not available on the AVR
-    };
+};
 
-    enum class Level : uint8_t {
+enum class Level : uint8_t {
     low = 0,
     high,
-    };
+};
 
-    struct GPIO_InitTypeDef {
+struct GPIO_InitTypeDef {
     Mode mode;
     Pull pull;
     Level level;
@@ -37,32 +37,32 @@ namespace gpio {
     inline GPIO_InitTypeDef(Mode mode, Level level)
         : mode(mode)
         , level(level) {};
-    };
+};
 
-    struct GPIO_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) {
+__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) {
+__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) {
+__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) {
+__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);
@@ -70,7 +70,7 @@ namespace gpio {
         portPin.port->DDRx &= ~(1 << portPin.pin);
         WritePin(portPin, (Level)GPIO_Init.pull);
     }
-    }
+}
 
 }
 }

src/hal/shr16.h

@@ -0,0 +1,55 @@
+#pragma once
+
+#include <stdint.h>
+
+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

src/hal/spi.h

@@ -6,13 +6,13 @@
 
 namespace hal {
 namespace spi {
-    struct SPI_TypeDef {
+struct SPI_TypeDef {
     volatile uint8_t SPCRx;
     volatile uint8_t SPSRx;
     volatile uint8_t SPDRx;
-    };
+};
 
-    struct SPI_InitTypeDef {
+struct SPI_InitTypeDef {
     hal::gpio::GPIO_pin miso_pin;
     hal::gpio::GPIO_pin mosi_pin;
     hal::gpio::GPIO_pin sck_pin;
@@ -20,9 +20,9 @@ namespace spi {
     uint8_t prescaler;
     uint8_t cpha;
     uint8_t cpol;
-    };
+};
 
-    __attribute__((always_inline)) inline void Init(SPI_TypeDef *const hspi, SPI_InitTypeDef *const conf) {
+__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));
@@ -34,14 +34,14 @@ namespace spi {
 
     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) {
+__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;
-    }
+}
 }
 }
 

src/hal/timers.h

@@ -5,10 +5,10 @@
 namespace hal {
 namespace timers {
 
-    /// timers
+/// timers
-    void ConfigureTimer(uint8_t timer /* some config struct */);
+void ConfigureTimer(uint8_t timer /* some config struct */);
-    void StartTimer(uint8_t timer);
+void StartTimer(uint8_t timer);
-    void StopTimer(uint8_t timer);
+void StopTimer(uint8_t timer);
 
 } // namespace cpu
 } // namespace hal

src/hal/usart.h

@@ -8,6 +8,8 @@
 /// for >1 USART interfaces
 
 namespace hal {
+namespace usart {
+
 class USART {
 public:
     struct USART_TypeDef {
@@ -81,8 +83,10 @@ public:
             husart->UCSRxB &= ~(1 << 5); // disable UDRE interrupt
     }
 
-    USART(USART_TypeDef *husart)
+    USART() = default;
-        : husart(husart) {};
+    void Init(USART_TypeDef *conf) {
+        husart = conf;
+    }
 
 private:
     // IO base address
@@ -93,7 +97,10 @@ private:
     CircleBuffer<uint8_t, 32> 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;

src/hal/watchdog.h

@@ -5,9 +5,9 @@
 namespace hal {
 namespace watchdog {
 
-    /// watchdog interface
+/// watchdog interface
-    void ConfigureWatchDog(uint16_t period);
+void ConfigureWatchDog(uint16_t period);
-    void ResetWatchDog();
+void ResetWatchDog();
 
 } // namespace watchdog
 } // namespace hal

src/main.cpp

@@ -1,27 +1,28 @@
-#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 "hal/shr16.h"
+
 #include "pins.h"
 #include <avr/interrupt.h>
 
-/// One-time setup of HW and SW components
+#include "modules/buttons.h"
-/// Called before entering the loop() function
+#include "modules/leds.h"
-void setup() {
+#include "modules/protocol.h"
+
+#include "logic/mm_control.h"
+
+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::SPI_InitTypeDef spi_conf = {
+    // SPI example
-    //     .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);
-
-    // // 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);
@@ -40,26 +41,94 @@ void setup() {
     //    gpio::WritePin(gpio::GPIO_pin(GPIOC, 6), gpio::Level::high);
     //    (void)dat;
 
-    USART::USART_InitTypeDef usart_conf = {
+    //    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();
+    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
+/// 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;
+
+    cpu::Init();
+
+    shr16::shr16.Init();
+    leds.SetMode(4, false, modules::leds::Mode::blink0);
+    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
+
+    hal::usart::USART::USART_InitTypeDef usart_conf = {
         .rx_pin = gpio::GPIO_pin(GPIOD, 2),
         .tx_pin = gpio::GPIO_pin(GPIOD, 3),
         .baudrate = 115200,
     };
+    hal::usart::usart1.Init(&usart_conf);
+    leds.SetMode(3, false, modules::leds::Mode::on);
+    leds.Step(0);
 
-    usart1.Init(&usart_conf);
+    // @@TODO if both shift register and the UART are dead, we are sitting ducks :(
-    sei();
+
-    usart1.puts("1234567890\n");
+    spi::SPI_InitTypeDef spi_conf = {
-    usart1.puts("1234567890\n");
+        .miso_pin = gpio::GPIO_pin(TMC2130_SPI_MISO_PIN),
-    usart1.puts("1234567890\n");
+        .mosi_pin = gpio::GPIO_pin(TMC2130_SPI_MOSI_PIN),
-    usart1.puts("1234567890\n");
+        .sck_pin = gpio::GPIO_pin(TMC2130_SPI_SCK_PIN),
-    usart1.puts("1234567890\n");
+        .ss_pin = gpio::GPIO_pin(TMC2130_SPI_SS_PIN),
-    usart1.puts("1234567890\n");
+        .prescaler = 2, //4mhz
-    usart1.puts("1234567890\n");
+        .cpha = 1,
-    usart1.puts("1234567890\n");
+        .cpol = 1,
-    usart1.puts("1234567890\n");
+    };
-    usart1.puts("1234567890\n");
+    spi::Init(SPI0, &spi_conf);
-    usart1.puts("1234567890\n");
+    leds.SetMode(2, false, modules::leds::Mode::on);
-    // usart1.Flush();
+    leds.Step(0);
+
+    // tmc::Init()
+    leds.SetMode(1, false, modules::leds::Mode::on);
+    leds.Step(0);
+
+    // adc::Init();
+    leds.SetMode(0, false, modules::leds::Mode::on);
+    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 (!hal::usart::usart1.ReadEmpty()) {
+        switch (protocol.DecodeRequest(hal::usart::usart1.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 +146,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));
+    leds.Step(0);
 }
 
 int main() {
     setup();
     for (;;) {
-        if (!usart1.ReadEmpty())
-            usart1.Write(usart1.Read());
         loop();
     }
     return 0;

src/modules/buttons.cpp

@@ -1,7 +1,7 @@
 #include "buttons.h"
-#include "../hal/adc.h"
 
 namespace modules {
+namespace buttons {
 
 uint16_t Buttons::tmpTiming = 0;
 
@@ -41,32 +41,31 @@ void Button::Step(uint16_t time, bool press) {
     }
 }
 
-int8_t Buttons::Sample() {
+int8_t Buttons::Sample(uint16_t rawADC) {
     // 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)
+    if (rawADC < 10)
         return 0;
-    else if (raw > 320 && raw < 360)
+    else if (rawADC > 320 && rawADC < 360)
         return 1;
-    else if (raw > 500 && raw < 530)
+    else if (rawADC > 500 && rawADC < 530)
         return 2;
     return -1;
 }
 
-void Buttons::Step() {
+void Buttons::Step(uint16_t rawADC) {
     // @@TODO temporary timing
     ++tmpTiming;
-    int8_t currentState = Sample();
+    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

src/modules/buttons.h

@@ -7,6 +7,7 @@
 /// This layer should contain debouncing of buttons and their logical interpretation
 
 namespace modules {
+namespace buttons {
 
 struct Button {
     inline constexpr Button()
@@ -56,7 +57,7 @@ public:
     inline constexpr Buttons() = default;
 
     /// State machine step - reads the ADC, processes debouncing, updates states of individual buttons
-    void Step();
+    void Step(uint16_t rawADC);
 
     /// @return true if button at index is pressed
     /// @@TODO add range checking if necessary
@@ -67,7 +68,8 @@ private:
 
     /// 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();
+    static int8_t Sample(uint16_t rawADC);
 };
 
+} // namespace buttons
 } // namespace modules

src/modules/leds.cpp

@@ -0,0 +1,53 @@
+#include "leds.h"
+#include "../hal/shr16.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;
+    }
+    return state.on;
+}
+
+void LEDs::Step(uint16_t delta_ms) {
+    ms += delta_ms;
+    bool oddPeriod = ((ms / 1000U) & 0x01U) != 0;
+    uint16_t result = 0;
+    for (int8_t i = ledPairs * 2 - 1; i >= 0; --i) {
+        result <<= 1;
+        result |= leds[i].Step(oddPeriod);
+    }
+
+    hal::shr16::shr16.SetLED(result);
+}
+
+} // namespace leds
+} // namespace modules

src/modules/leds.h

@@ -1,3 +1,95 @@
 #pragma once
 
-/// @@TODO @leptun design some nice API ;)
+#include <stdint.h>
+
+/// 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.
+///
+/// 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
+};
+
+enum Color {
+    green = 0,
+    red = 1
+};
+
+/// 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
+    void Step(uint16_t delta_ms);
+
+    inline constexpr uint8_t LedPairsCount() const { return ledPairs; }
+
+    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);
+    }
+
+    inline bool LedOn(uint8_t index) const {
+        return leds[index].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;
+};
+
+} // namespace LEDs
+} // namespace modules

src/modules/protocol.cpp

@@ -11,6 +11,7 @@
 //  <OID> 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
@@ -24,7 +25,7 @@ namespace modules {
 // msgvalue  0-9 ->msgvalue
 //           \n ->start successfully accepted command
 
-Protocol::DecodeStatus Protocol::DecodeRequest(uint8_t c) {
+DecodeStatus Protocol::DecodeRequest(uint8_t c) {
     switch (rqState) {
     case RequestStates::Code:
         switch (c) {
@@ -81,7 +82,7 @@ uint8_t Protocol::EncodeRequest(const RequestMsg &msg, uint8_t *txbuff) {
     return 3;
 }
 
-Protocol::DecodeStatus Protocol::DecodeResponse(uint8_t c) {
+DecodeStatus Protocol::DecodeResponse(uint8_t c) {
     switch (rspState) {
     case ResponseStates::RequestCode:
         switch (c) {
@@ -218,4 +219,5 @@ uint8_t Protocol::EncodeResponseQueryOperation(const RequestMsg &msg, ResponseMs
     return dst - txbuff + 1;
 }
 
+} // namespace protocol
 } // namespace modules

src/modules/protocol.h

@@ -7,6 +7,7 @@
 /// @@TODO possibly add some checksum to verify the correctness of messages
 
 namespace modules {
+namespace protocol {
 
 enum class RequestMsgCodes : uint8_t {
     unknown = 0,
@@ -56,18 +57,18 @@ struct ResponseMsg {
         , 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:
-    /// 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
-    };
-
     inline Protocol()
         : rqState(RequestStates::Code)
         , requestMsg(RequestMsgCodes::unknown, 0)
@@ -140,4 +141,5 @@ private:
     ResponseMsg responseMsg;
 };
 
+} // namespace protocol
 } // namespace modules

tests/unit/modules/CMakeLists.txt

@@ -1,2 +1,3 @@
 add_subdirectory(buttons)
+add_subdirectory(leds)
 add_subdirectory(protocol)

tests/unit/modules/buttons/stub_adc.cpp

@@ -3,22 +3,22 @@
 #include <vector>
 
 namespace hal {
-namespace ADC {
+namespace adc {
 
-    static TADCData values2Return;
+static TADCData values2Return;
-    static TADCData::const_iterator rdptr = values2Return.cbegin();
+static TADCData::const_iterator rdptr = values2Return.cbegin();
-    static uint8_t oversampleFactor = 1;
+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 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) {
+void ReinitADC(TADCData &&d, uint8_t ovsmpl) {
     values2Return = std::move(d);
     oversampleFactor = ovsmpl;
     oversample = ovsmpl;
     rdptr = values2Return.cbegin();
-    }
+}
 
-    /// ADC access routines
+/// ADC access routines
-    uint16_t ReadADC(uint8_t /*adc*/) {
+uint16_t ReadADC(uint8_t /*adc*/) {
     if (!oversample) {
         ++rdptr;
         oversample = oversampleFactor;
@@ -26,7 +26,7 @@ namespace ADC {
         --oversample;
     }
     return rdptr != values2Return.end() ? *rdptr : 1023;
-    }
+}
 
-} // namespace ADC
+} // namespace adc
 } // namespace hal

tests/unit/modules/buttons/stub_adc.h

@@ -4,11 +4,11 @@
 #include <vector>
 
 namespace hal {
-namespace ADC {
+namespace adc {
 
-    using TADCData = std::vector<uint16_t>;
+using TADCData = std::vector<uint16_t>;
 
-    void ReinitADC(TADCData &&d, uint8_t ovsmpl);
+void ReinitADC(TADCData &&d, uint8_t ovsmpl);
 
-} // namespace ADC
+} // namespace adc
 } // namespace hal

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) {
+bool Step_Basic_One_Button(hal::adc::TADCData &&d, uint8_t testedButtonIndex) {
-    using namespace modules;
+    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;
+    using namespace modules::buttons;
-    hal::ADC::TADCData d({ 5, 6, 1023, 321, 359, 1023, 501, 529, 1023 });
+    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));

tests/unit/modules/leds/CMakeLists.txt

@@ -0,0 +1,10 @@
+# define the test executable
+add_executable(leds_tests ../../../../src/modules/leds.cpp stub_shr16.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)

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

tests/unit/modules/leds/test_leds.cpp

@@ -0,0 +1,113 @@
+#include "catch2/catch.hpp"
+#include "leds.h"
+#include "shr16.h"
+
+using Catch::Matchers::Equals;
+
+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);
+}

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_[0]) == DecodeStatus::NeedMoreData);
-    CHECK(p.DecodeRequest(B1_[1]) == Protocol::DecodeStatus::NeedMoreData);
+    CHECK(p.DecodeRequest(B1_[1]) == DecodeStatus::NeedMoreData);
-    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);
 
     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;
         }
     });