Rebase onto main + clean up the code a bit

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
+  firmware
-                   src/modules/buttons.cpp src/modules/leds.cpp
+  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(

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/avr/usart.cpp

@@ -1,72 +1,82 @@
 #include "../usart.h"
 #include <avr/interrupt.h>
 
-uint8_t hal::USART::Read() {
+namespace hal {
-    uint8_t c = 0;
+namespace usart {
-    this->rx_buf.ConsumeFirst(c);
-    return c;
-}
 
-void hal::USART::Write(uint8_t c) {
+    USART usart1;
-    _written = true;
+
-    // If the buffer and the data register is empty, just write the byte
+    uint8_t USART::Read() {
-    // to the data register and be done. This shortcut helps
+        uint8_t c = 0;
-    // significantly improve the effective datarate at high (>
+        rx_buf.ConsumeFirst(c);
-    // 500kbit/s) bitrates, where interrupt overhead becomes a slowdown.
+        return c;
-    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
+    void USART::Write(uint8_t c) {
-    // wait for the interrupt handler to empty it a bit
+        _written = true;
-    while (!tx_buf.push_back_DontRewrite(c)) {
+        // If the buffer and the data register is empty, just write the byte
-        if (bit_is_clear(SREG, SREG_I)) {
+        // to the data register and be done. This shortcut helps
-            // Interrupts are disabled, so we'll have to poll the data
+        // significantly improve the effective datarate at high (>
-            // register empty flag ourselves. If it is set, pretend an
+        // 500kbit/s) bitrates, where interrupt overhead becomes a slowdown.
-            // interrupt has happened and call the handler to free up
+        if (tx_buf.IsEmpty() && (husart->UCSRxA & (1 << 5))) {
-            // space for us.
+            husart->UDRx = c;
-            if (husart->UCSRxA & (1 << 5))
+            husart->UCSRxA |= (1 << 6);
-                ISR_UDRE();
+            return;
-        } 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();
+                }
+            } 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
+} // namespace usart
-}
+} // namespace hal
-
-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);
 
 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/usart.h

@@ -8,92 +8,99 @@
 /// for >1 USART interfaces
 
 namespace hal {
-class USART {
+namespace 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 {
+    class USART {
-        hal::gpio::GPIO_pin rx_pin;
+    public:
-        hal::gpio::GPIO_pin tx_pin;
+        struct USART_TypeDef {
-        uint32_t baudrate;
+            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
+        struct USART_InitTypeDef {
-    uint8_t Peek() const {
+            hal::gpio::GPIO_pin rx_pin;
-        return rx_buf.GetFirstIfAble();
+            hal::gpio::GPIO_pin tx_pin;
-    }
+            uint32_t baudrate;
-    /// @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)
+        /// @returns current character from the UART without extracting it from the read buffer
-    void Write(uint8_t c);
+        uint8_t Peek() const {
-    /// @param str c string to be sent. NL is appended
+            return rx_buf.GetFirstIfAble();
-    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 true if there are no bytes to be read
-    /// implementation of the transmit ISR's body
+        bool ReadEmpty() const {
-    __attribute__((always_inline)) inline void ISR_UDRE() {
+            return rx_buf.IsEmpty();
-        uint8_t c = 0;
+        }
-        tx_buf.ConsumeFirst(c);
+        /// @returns current character from the UART and extracts it from the read buffer
-        husart->UDRx = c;
+        uint8_t Read();
 
-        // clear the TXC bit -- "can be cleared by writing a one to its bit
+        /// @param c character to be pushed into the TX buffer (to be sent)
-        // location". This makes sure flush() won't return until the bytes
+        void Write(uint8_t c);
-        // actually got written
+        /// @param str c string to be sent. NL is appended
-        husart->UCSRxA |= (1 << 6);
+        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())
+        /// Initializes USART interface
-            husart->UCSRxB &= ~(1 << 5); // disable UDRE interrupt
+        __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)
+        /// implementation of the receive ISR's body
-        : husart(husart) {};
+        __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:
+            // clear the TXC bit -- "can be cleared by writing a one to its bit
-    // IO base address
+            // location". This makes sure flush() won't return until the bytes
-    USART_TypeDef *husart;
+            // actually got written
-    bool _written;
+            husart->UCSRxA |= (1 << 6);
 
-    CircleBuffer<uint8_t, 32> tx_buf;
+            if (tx_buf.IsEmpty())
-    CircleBuffer<uint8_t, 32> rx_buf;
+                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<uint8_t, 32> tx_buf;
+        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/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();
+    sei();
-    usart1.puts("1234567890\n");
+    hal::usart::usart1.puts("1234567890\n");
-    usart1.puts("1234567890\n");
+    hal::usart::usart1.puts("1234567890\n");
-    usart1.puts("1234567890\n");
+    hal::usart::usart1.puts("1234567890\n");
-    usart1.puts("1234567890\n");
+    hal::usart::usart1.puts("1234567890\n");
-    usart1.puts("1234567890\n");
+    hal::usart::usart1.puts("1234567890\n");
-    usart1.puts("1234567890\n");
+    hal::usart::usart1.puts("1234567890\n");
-    usart1.puts("1234567890\n");
+    hal::usart::usart1.puts("1234567890\n");
-    usart1.puts("1234567890\n");
+    hal::usart::usart1.puts("1234567890\n");
-    usart1.puts("1234567890\n");
+    hal::usart::usart1.puts("1234567890\n");
-    usart1.puts("1234567890\n");
+    hal::usart::usart1.puts("1234567890\n");
-    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()) {
+    while (!hal::usart::usart1.ReadEmpty()) {
-        switch (protocol.DecodeRequest(uart.Read())) {
+        switch (protocol.DecodeRequest(hal::usart::usart1.Read())) {
         case mpd::MessageCompleted:
             // process the input message
             return true;