Better hardware abstraction using C++20 concepts

CMakeLists.txt

@@ -5,7 +5,7 @@ set(CMAKE_BUILD_TYPE Release CACHE STRING "Choose Debug or Release")
 project(pinetime VERSION 1.11.0 LANGUAGES C CXX ASM)
 
 set(CMAKE_C_STANDARD 99)
-set(CMAKE_CXX_STANDARD 14)
+set(CMAKE_CXX_STANDARD 20)
 
 # set(CMAKE_GENERATOR "Unix Makefiles")
 set(CMAKE_C_EXTENSIONS OFF)

README.md

@@ -23,7 +23,7 @@ Fast open-source firmware for the [PineTime smartwatch](https://www.pine64.org/p
 ## Development
 
 - [InfiniTime Vision](doc/InfiniTimeVision.md)
-- [Rough structure of the code](doc/code/Intro.md)
+- [Introduction to the code](doc/code/Intro.md)
 - [How to implement an application](doc/code/Apps.md)
 - [Generate the fonts and symbols](src/displayapp/fonts/README.md)
 - [Tips on designing an app UI](doc/ui_guidelines.md)

doc/code/Intro.md

@@ -47,3 +47,88 @@ For more detail see the [Apps page](./Apps.md)
 ## Bluetooth
 
 Header files with short documentation for the functions are inside [libs/mynewt-nimble/nimble/host/include/host/](/src/libs/mynewt-nimble/nimble/host/include/host/).
+
+## Hardware abstraction and device drivers
+
+Until version 1.12, InfiniTime did not provide any kind of hardware abstraction : the drivers were written specifically for the PineTime, and there was no easy way to provide any alternative implementation to support variants of the PineTime or for the integration in [InfiniSim](https://github.com/InfiniTimeOrg/InfiniSim).
+
+In [InfiniTime 1.12](https://github.com/InfiniTimeOrg/InfiniTime/releases/tag/1.12.0), we implemented a new design that allows to easily choose **at build time** a specific implementation for multiple device drivers (Display, heart rate sensor, motion sensor, SPI, flash memory, touch panel, TWI and Watchdog).
+
+This new design makes the code much cleaner in InfiniSim and allows the port of InfiniTime on other hardware (ex : many PineTime variants like the Colmi P8) more easily.
+
+This hardware abstraction is based on C++ `concepts` and a proxy object that enables 'static polymorphism'. It means that the actual implementations of the drivers are known at **build time** and that there's no `virtual` calls at runtime. 
+
+Here's an overview of the design :
+
+```c++
+namespace Pinetime {
+  namespace Drivers  {
+    template <typename DeviceImpl>
+    concept IsDevice = { /* ... */ };
+
+    namespace Interface {
+      template <class T>
+        requires IsDevice<T>
+      class Device {
+      public:
+         explicit Device(T& impl) : impl {impl} {}
+      /* ... */
+      private:
+        T& impl;
+      };
+    }
+
+    using Device = Interface::Device<Pinetime::Drivers::SomeDevice::Device>;
+  }
+}
+
+int main() {
+  /* ... */
+
+  Pinetime::Drivers::Category::Device deviceImpl { /* ctor arguments specific to this implementation of Device */ };
+  Pinetime::Drivers::Device device {deviceImpl};
+
+  /* ... */
+}
+
+```
+
+The concept `Pinetime::Drivers::IsDevice` describes the interface a class that implements a `Device` must expose.
+
+The template class `Pinetime::Drivers::Interface::Device` is the "proxy" objects that allows the build time polymorphism.
+
+`Pinetime::Drivers::Device` is aliased to `Pinetime::Drivers::Interface::Device`. This allows to remove the template argument so that the rest of the application does not need to handle it. Those aliases are defined in header files located in `port/`. This is the only place where `#ifdef`s are needed.
+
+The actual drivers are implemented in specific namespaces (`Pinetime::Drivers::MCU::Device` or `Pinetime::Drivers::Category::Device`).
+
+To declare a new driver in the code, you'll first need to instantiate an actual implementation of the driver and then give the reference to this instance to the corresponding proxy object. Here is an example with the display driver:
+
+```c++
+// Actual implementation of the SPI bus for the NRF52 MCU
+Pinetime::Drivers::Nrf52::SpiMaster spiImpl {Pinetime::Drivers::Nrf52::SpiMaster::SpiModule::SPI0,
+                                  {Pinetime::Drivers::Nrf52::SpiMaster::BitOrder::Msb_Lsb,
+                                   Pinetime::Drivers::Nrf52::SpiMaster::Modes::Mode3,
+                                   Pinetime::Drivers::Nrf52::SpiMaster::Frequencies::Freq8Mhz,
+                                   Pinetime::PinMap::SpiSck,
+                                   Pinetime::PinMap::SpiMosi,
+                                   Pinetime::PinMap::SpiMiso}};
+
+// Proxy object 
+Pinetime::Drivers::SpiMaster spi {spiImpl};
+
+// Actual implementation of the SpiMaster drivers (it handles the chip select pin)
+Pinetime::Drivers::Nrf52::Spi lcdSpiIpmpl {spiImpl, Pinetime::PinMap::SpiLcdCsn};
+
+// Proxy object
+Pinetime::Drivers::Spi lcdSpi {lcdSpiIpmpl};
+
+// Actual implementation of the display driver (ST7789 display controller)
+Pinetime::Drivers::Displays::St7789 lcdImpl {lcdSpi, Pinetime::PinMap::LcdDataCommand};
+
+// Proxy object
+Pinetime::Drivers::Display lcd{lcdImpl};
+
+Pinetime::System::SystemTask systemTask(spi, lcd /* ... */);
+```
+
+Once the initialization is done, the application does not need to know the actual implementation of the drivers, all calls to the drivers will go through the proxy objects. 

src/CMakeLists.txt

@@ -435,15 +435,16 @@ list(APPEND SOURCE_FILES
         ##
 
         main.cpp
-        drivers/St7789.cpp
-        drivers/SpiNorFlash.cpp
-        drivers/SpiMaster.cpp
-        drivers/Spi.cpp
-        drivers/Watchdog.cpp
+        drivers/displays/St7789.cpp
+        drivers/nrf52/SpiMaster.cpp
+        drivers/nrf52/Spi.cpp
+        drivers/nrf52/TwiMaster.cpp
+        drivers/spiFlash/SpiNorFlash.cpp
+        drivers/nrf52/Watchdog.cpp
         drivers/DebugPins.cpp
         drivers/InternalFlash.cpp
-        drivers/Hrs3300.cpp
-        drivers/Bma421.cpp
+        drivers/heartRateSensors/Hrs3300.cpp
+        drivers/motionSensors/Bma421.cpp
         drivers/Bma421_C/bma4.c
         drivers/Bma421_C/bma423.c
         components/battery/BatteryController.cpp
@@ -474,7 +475,7 @@ list(APPEND SOURCE_FILES
         components/timer/TimerController.cpp
         components/alarm/AlarmController.cpp
         components/fs/FS.cpp
-        drivers/Cst816s.cpp
+        drivers/touchpanels/Cst816s.cpp
         FreeRTOS/port.c
         FreeRTOS/port_cmsis_systick.c
         FreeRTOS/port_cmsis.c
@@ -484,7 +485,6 @@ list(APPEND SOURCE_FILES
 
         systemtask/SystemTask.cpp
         systemtask/SystemMonitor.cpp
-        drivers/TwiMaster.cpp
 
         heartratetask/HeartRateTask.cpp
         components/heartrate/Ppg.cpp
@@ -502,15 +502,16 @@ list(APPEND RECOVERY_SOURCE_FILES
         displayapp/DisplayAppRecovery.cpp
 
         main.cpp
-        drivers/St7789.cpp
-        drivers/SpiNorFlash.cpp
-        drivers/SpiMaster.cpp
-        drivers/Spi.cpp
-        drivers/Watchdog.cpp
+        drivers/displays/St7789.cpp
+        drivers/nrf52/SpiMaster.cpp
+        drivers/nrf52/Spi.cpp
+        drivers/nrf52/TwiMaster.cpp
+        drivers/spiFlash/SpiNorFlash.cpp
+        drivers/nrf52/Watchdog.cpp
         drivers/DebugPins.cpp
         drivers/InternalFlash.cpp
-        drivers/Hrs3300.cpp
-        drivers/Bma421.cpp
+        drivers/heartRateSensors/Hrs3300.cpp
+        drivers/motionSensors/Bma421.cpp
         drivers/Bma421_C/bma4.c
         drivers/Bma421_C/bma423.c
         components/battery/BatteryController.cpp
@@ -539,14 +540,13 @@ list(APPEND RECOVERY_SOURCE_FILES
         components/settings/Settings.cpp
         components/timer/TimerController.cpp
         components/alarm/AlarmController.cpp
-        drivers/Cst816s.cpp
+        drivers/touchpanels/Cst816s.cpp
         FreeRTOS/port.c
         FreeRTOS/port_cmsis_systick.c
         FreeRTOS/port_cmsis.c
 
         systemtask/SystemTask.cpp
         systemtask/SystemMonitor.cpp
-        drivers/TwiMaster.cpp
         components/gfx/Gfx.cpp
         components/rle/RleDecoder.cpp
         components/heartrate/HeartRateController.cpp
@@ -566,15 +566,16 @@ list(APPEND RECOVERYLOADER_SOURCE_FILES
         FreeRTOS/port_cmsis_systick.c
         FreeRTOS/port_cmsis.c
 
-        drivers/SpiNorFlash.cpp
-        drivers/SpiMaster.cpp
-        drivers/Spi.cpp
+        drivers/nrf52/SpiMaster.cpp
+        drivers/nrf52/Spi.cpp
+        drivers/nrf52/TwiMaster.cpp
+        drivers/spiFlash/SpiNorFlash.cpp
         logging/NrfLogger.cpp
 
         components/rle/RleDecoder.cpp
 
         components/gfx/Gfx.cpp
-        drivers/St7789.cpp
+        drivers/displays/St7789.cpp
         components/brightness/BrightnessController.cpp
 
         recoveryLoader.cpp
@@ -617,16 +618,18 @@ set(INCLUDE_FILES
         displayapp/widgets/Counter.h
         displayapp/widgets/PageIndicator.h
         displayapp/widgets/StatusIcons.h
-        drivers/St7789.h
+        drivers/displays/St7789.h
         drivers/SpiNorFlash.h
-        drivers/SpiMaster.h
-        drivers/Spi.h
+        drivers/nrf52/SpiMaster.h
+        drivers/nrf52/Spi.h
+        drivers/nrf52/TwiMaster.h
+        drivers/spiFlash/SpiNorFlash.h
         drivers/Watchdog.h
         drivers/DebugPins.h
         drivers/InternalFlash.h
-        drivers/Hrs3300.h
+        drivers/heartRateSensors/Hrs3300.h
         drivers/PinMap.h
-        drivers/Bma421.h
+        drivers/motionSensors/Bma421.h
         drivers/Bma421_C/bma4.c
         drivers/Bma421_C/bma423.c
         components/battery/BatteryController.h
@@ -655,7 +658,7 @@ set(INCLUDE_FILES
         components/settings/Settings.h
         components/timer/TimerController.h
         components/alarm/AlarmController.h
-        drivers/Cst816s.h
+        drivers/touchpanels/Cst816s.h
         FreeRTOS/portmacro.h
         FreeRTOS/portmacro_cmsis.h
         libs/date/include/date/tz.h
@@ -687,6 +690,7 @@ include_directories(
         .
         ../
         libs/
+        port/
         FreeRTOS/
         libs/date/include
         libs/mynewt-nimble/porting/npl/freertos/include

src/components/ble/DfuService.h

@@ -9,13 +9,12 @@
 #undef max
 #undef min
 
+#include "port/SpiNorFlash.h"
+
 namespace Pinetime {
   namespace System {
     class SystemTask;
   }
-  namespace Drivers {
-    class SpiNorFlash;
-  }
   namespace Controllers {
     class Ble;
 

src/components/ble/NimbleController.h

@@ -25,10 +25,6 @@
 #include "components/fs/FS.h"
 
 namespace Pinetime {
-  namespace Drivers {
-    class SpiNorFlash;
-  }
-
   namespace System {
     class SystemTask;
   }

src/components/fs/FS.h

@@ -2,6 +2,7 @@
 
 #include <cstdint>
 #include "drivers/SpiNorFlash.h"
+#include "port/SpiNorFlash.h"
 #include <littlefs/lfs.h>
 
 namespace Pinetime {

src/components/gfx/Gfx.cpp

@@ -1,8 +1,8 @@
 #include "components/gfx/Gfx.h"
-#include "drivers/St7789.h"
+#include "drivers/displays/St7789.h"
 using namespace Pinetime::Components;
 
-Gfx::Gfx(Pinetime::Drivers::St7789& lcd) : lcd {lcd} {
+Gfx::Gfx(Pinetime::Drivers::Displays::St7789& lcd) : lcd {lcd} {
 }
 
 void Gfx::Init() {

src/components/gfx/Gfx.h

@@ -8,12 +8,14 @@
 
 namespace Pinetime {
   namespace Drivers {
-    class St7789;
+    namespace Displays {
+      class St7789;
+    }
   }
   namespace Components {
     class Gfx : public Pinetime::Drivers::BufferProvider {
     public:
-      explicit Gfx(Drivers::St7789& lcd);
+      explicit Gfx(Drivers::Displays::St7789& lcd);
       void Init();
       void ClearScreen();
       void DrawString(uint8_t x, uint8_t y, uint16_t color, const char* text, const FONT_INFO* p_font, bool wrap);
@@ -49,7 +51,7 @@ namespace Pinetime {
       volatile State state;
 
       uint16_t buffer[width]; // 1 line buffer
-      Drivers::St7789& lcd;
+      Drivers::Displays::St7789& lcd;
 
       void SetBackgroundColor(uint16_t color);
       void WaitTransferFinished() const;

src/components/motion/MotionController.cpp

@@ -69,12 +69,12 @@ int32_t MotionController::currentShakeSpeed() {
 void MotionController::IsSensorOk(bool isOk) {
   isSensorOk = isOk;
 }
-void MotionController::Init(Pinetime::Drivers::Bma421::DeviceTypes types) {
+void MotionController::Init(Pinetime::Drivers::MotionSensors::DeviceTypes types) {
   switch (types) {
-    case Drivers::Bma421::DeviceTypes::BMA421:
+    case Drivers::MotionSensors::DeviceTypes::BMA421:
       this->deviceType = DeviceTypes::BMA421;
       break;
-    case Drivers::Bma421::DeviceTypes::BMA425:
+    case Drivers::MotionSensors::DeviceTypes::BMA425:
       this->deviceType = DeviceTypes::BMA425;
       break;
     default:

src/components/motion/MotionController.h

@@ -1,7 +1,8 @@
 #pragma once
 
 #include <cstdint>
-#include <drivers/Bma421.h>
+#include <drivers/MotionSensor.h>
+#include <port/MotionSensor.h>
 #include <components/ble/MotionService.h>
 
 namespace Pinetime {
@@ -48,7 +49,7 @@ namespace Pinetime {
         return deviceType;
       }
 
-      void Init(Pinetime::Drivers::Bma421::DeviceTypes types);
+      void Init(Pinetime::Drivers::MotionSensors::DeviceTypes types);
       void SetService(Pinetime::Controllers::MotionService* service);
 
     private:

src/displayapp/DisplayApp.cpp

@@ -30,9 +30,6 @@
 #include "displayapp/screens/PassKey.h"
 #include "displayapp/screens/Error.h"
 
-#include "drivers/Cst816s.h"
-#include "drivers/St7789.h"
-#include "drivers/Watchdog.h"
 #include "systemtask/SystemTask.h"
 #include "systemtask/Messages.h"
 
@@ -60,9 +57,9 @@ namespace {
   }
 }
 
-DisplayApp::DisplayApp(Drivers::St7789& lcd,
+DisplayApp::DisplayApp(Drivers::Display& lcd,
                        Components::LittleVgl& lvgl,
-                       Drivers::Cst816S& touchPanel,
+                       Pinetime::Drivers::TouchPanel& touchPanel,
                        Controllers::Battery& batteryController,
                        Controllers::Ble& bleController,
                        Controllers::DateTime& dateTimeController,