[ISSUE-20]: post-merge refinement

Author: SzigetiJ

README.md

@@ -89,6 +89,7 @@ THis list is supposed to grow longer and longer.
 * Device drivers
   * BME280 Temperature / Pressure / Humidity sensor
   * (_TODO_: cleanup) BH1750 Light sensor
+  * WS2812B LED strip
 * etc (_TODO_)
 
 ### Simplifications

examples/1rmtmorse/README.md

@@ -66,7 +66,7 @@ ESP32.GPIO2 -- R1 --(A) D1 (K)-- ESP32.GND
 1. Play around with the memory size allocated to the RMT channel.
 
 2. The ESP32 reference manual states that in `TX_CONTI_MODE`
-"there will be an idle level lasting one clk_div cycle between N and N+1 transmissions."
+"there will be an idle level lasting one `clk_div` cycle between N and N+1 transmissions."
 Solve this problem somehow.
 
 3. Instead of periodic register scanning, use an ISR.
@@ -75,6 +75,6 @@ Solve this problem somehow.
 (transition start should be triggered by newline character).
 
 5. Replace D1 with a speaker and make audio output.
-To achieve this you have to modulate the RMT pulses with a _carrier_ wave.
+To achieve this you have to modulate the RMT pulses with a *carrier* wave.
 The setup of the carrier is already implemented, you just have to switch on (set `CARRIER_EN` to `1`).
 What you still can do is playing around with the carrier frequency / duty cycle.

examples/1rmtmusic/rmtmusic.c

@@ -265,7 +265,7 @@ static void _rmtmusic_init() {
 
   // register ISR and enable it
   rmt_isr_init();
-  rmt_isr_register(RMTMUSIC_CH, NULL, NULL, _rmtmusic_feed, NULL, &gsMusicState);
+  rmt_isr_register(RMTMUSIC_CH, RMT_INT_TXTHRES, _rmtmusic_feed, &gsMusicState);
   rmt_isr_start(CPU_PRO, RMTINT_CH);
 }
 

examples/1rmtws2812/rmtws2812.c

@@ -88,8 +88,8 @@ const Color gasStops[] = {
 static uint8_t gau8PreBuffer0[3 * STRIP_LENGTH];
 static uint8_t gau8PreBuffer1[3 * STRIP_LENGTH];
 static uint8_t gau8Buffer[3 * STRIP_LENGTH];
-static const SWs2812Iface gsWs2812Iface = {.eChannel = RMTWS2812_CH, .u8Blocks = RMTWS2812_MEM_BLOCKS};
-static SWs2812FeederState gsFeederState = {.sIface = gsWs2812Iface, .pu8Data = gau8Buffer, .szLen = sizeof (gau8Buffer), .szPos = 0};
+static SWs2812State gsFeederState;
+static bool gbFeederBusy = false;
 
 // ==================== Implementation ================
 static void _fill_prebuffer(uint8_t *pu8Dest, uint8_t u8Stop0Idx, uint8_t u8Stop1Idx) {
@@ -128,8 +128,8 @@ static uint8_t *_rotbuf_weighted_avg(uint8_t *pu8Res, uint32_t u32Len, uint8_t *
 }
 
 IRAM_ATTR static void _rmtws2812_txend_cb(void *pvParam) {
-  SWs2812FeederState *psParam = (SWs2812FeederState*)pvParam;
-  psParam->bBusy = false;
+  bool *psParam = (bool*)pvParam;
+  *psParam = false;
 }
 
 static void _rmtws2812_init_data() {
@@ -140,7 +140,8 @@ static void _rmtws2812_init_data() {
 static void _rmtws2812_init_peripheral() {
   rmt_isr_init(); // ws2812_init will write into rmt_isr table
   rmt_init_controller(true, true);
-  ws2812_init(RMTWS2812_GPIO, APB_FREQ_HZ, _rmtws2812_txend_cb, &gsFeederState);
+  gsFeederState = ws2812_init_feederstate(gau8Buffer, sizeof (gau8Buffer), RMTWS2812_CH, RMTWS2812_MEM_BLOCKS);
+  ws2812_init(RMTWS2812_GPIO, APB_FREQ_HZ, &gsFeederState, _rmtws2812_txend_cb, &gbFeederBusy);
 
   // enable RMT ISR
   rmt_isr_start(CPU_PRO, RMTINT_CH);
@@ -151,6 +152,7 @@ static void _rmtws2812_cycle(uint64_t u64Ticks) {
   static bool bFirstRun = true;
 
   if (bFirstRun) {
+    gbFeederBusy = true;
     ws2812_start(&gsFeederState);
     bFirstRun = false;
   }
@@ -160,8 +162,8 @@ static void _rmtws2812_cycle(uint64_t u64Ticks) {
       gpsRMT->arInt[RMT_INT_CLR] = rmt_int_bit(RMTWS2812_CH, RMT_INT_TXEND);
       gsUART0.FIFO = 'E';
     }
-    if (gsFeederState.szPos == gsFeederState.szLen && !gsFeederState.bBusy) {
-      gsFeederState.szPos = 0;
+    if (gsFeederState.szPos == gsFeederState.szLen && !gbFeederBusy) {
+      gbFeederBusy = true;
       ws2812_start(&gsFeederState);
     }
     if (gpsRMT->arInt[RMT_INT_ST] & rmt_int_bit(RMTWS2812_CH, RMT_INT_ERR)) {

modules/bme280.c

@@ -92,16 +92,16 @@ typedef union {
 
   struct {
     bool bDirtyCtrlHum : 1;
-    bool bDirtyStatus : 1; // meaning that the status bye must be read
-    bool bDirtyCtrlMeas : 1; // note: async comm is not triggered until bModeSet is set.
+    bool bDirtyStatus : 1;    // meaning that the status bye must be read
+    bool bDirtyCtrlMeas : 1;  // note: async comm is not triggered until bModeSet is set.
     bool bDirtyConfig : 1;
     bool bDataUpdated : 1;
     bool bCalib0Ready : 1;
     bool bCalib1Ready : 1;
     bool bWaitingForRx : 1;
-    bool bModeSet : 1; // triggers chain of state changes until the mode bits get written
+    bool bModeSet : 1;        // triggers chain of state changes until the mode bits get written
     bool bRequestForData : 1; // triggers chain of state changes until data bytes are read out
-    bool bReset : 1;  // triggers write to reset register
+    bool bReset : 1;          // triggers write to reset register
     uint32_t rsvd11 : 5;
     uint8_t u8CurAddr : 8;
     uint8_t u5CurLen : 5;
@@ -496,7 +496,7 @@ bool bme280_async_tx_cycle(const SI2cIfaceCfg *psIface, SBme280StateDesc *psStat
     } else if (!psFlags->bCalib1Ready) {
       _read_bytes(psIface, psEntry, psState, psState->au8Calib + MEMLEN_CALIB0, MEMADDR_CALIB1, MEMLEN_CALIB1);
     } else if (psFlags->bDirtyStatus) {
-      _read_bytes(psIface, psEntry, psState, psState->au8Config + MEMADDR_STATUS - MEMADDR_CTRLH, MEMADDR_STATUS, 1);
+      _read_bytes(psIface, psEntry, psState, psState->au8Config + (MEMADDR_STATUS - MEMADDR_CTRLH), MEMADDR_STATUS, 1);
     } else if (!psFlags->bDataUpdated) {
       _read_bytes(psIface, psEntry, psState, psState->au8Data, MEMADDR_DATA, MEMLEN_DATA);
     } else {

modules/bme280.h

@@ -73,9 +73,9 @@ extern "C" {
     uint32_t u32LastLabel;
     uint32_t u32CommState;
     // the following attributes are storing data trasmitted to / received from the target device
-    uint8_t au8Calib[42]; ///< bytes at mem 0x88 .. 0xa1, 0xe1 .. 0xf0
-    uint8_t au8Data[8]; ///< bytes at mem 0xf7 .. 0xfe
-    uint8_t au8Config[4]; ///< bytes at mem 0xf2 .. 0xf5
+    uint8_t au8Calib[42];   ///< bytes at mem 0x88 .. 0xa1, 0xe1 .. 0xf0
+    uint8_t au8Data[8];     ///< bytes at mem 0xf7 .. 0xfe
+    uint8_t au8Config[4];   ///< bytes at mem 0xf2 .. 0xf5
   } SBme280StateDesc;
 
   // interface functions

modules/ws2812.c

@@ -21,9 +21,9 @@
 #define RMT_CLK_NS            (1000000 / RMT_FREQ_KHZ)
 
 // ================ Local function declarations =================
-static void _rmt_config_channel(SWs2812Iface *psIface, uint8_t u8Divisor);
+static void _rmt_config_channel(const SWs2812Iface *psIface, uint8_t u8Divisor);
 void _byte_to_rmtram(RegAddr prDest, uint8_t u8Value);
-static bool _put_next_byte(SWs2812FeederState *psState);
+static bool _put_next_byte(SWs2812State *psState);
 static void _feeder(void *pvParam);
 
 // =================== Global constants ================
@@ -46,7 +46,8 @@ const uint32_t *pu32EntryPair = (const uint32_t*)gau16Entries;
 // ==================== Local Data ================
 
 // ==================== Implementation ================
-static void _rmt_config_channel(SWs2812Iface *psIface, uint8_t u8Divisor) {
+
+static void _rmt_config_channel(const SWs2812Iface *psIface, uint8_t u8Divisor) {
   // rmt channel config
   SRmtChConf rChConf = {
     .r0 =
@@ -73,7 +74,7 @@ IRAM_ATTR void _byte_to_rmtram(RegAddr prDest, uint8_t u8Value) {
   }
 }
 
-IRAM_ATTR static bool _put_next_byte(SWs2812FeederState *psState) {
+IRAM_ATTR static bool _put_next_byte(SWs2812State *psState) {
   uint32_t u32Offset = 8 * psState->szPos;
   RegAddr prDest = rmt_ram_addr(psState->sIface.eChannel, psState->sIface.u8Blocks, u32Offset);
   if (psState->szPos < psState->szLen) {
@@ -86,7 +87,7 @@ IRAM_ATTR static bool _put_next_byte(SWs2812FeederState *psState) {
 }
 
 static void _feeder(void *pvParam) {
-  SWs2812FeederState *psParam = (SWs2812FeederState*)pvParam;
+  SWs2812State *psParam = (SWs2812State*)pvParam;
 
   for (int i = 0; i < (psParam->sIface.u8Blocks * RMT_RAM_BLOCK_SIZE) / 16; ++i) {
     if (!_put_next_byte(psParam)) break;
@@ -95,16 +96,17 @@ static void _feeder(void *pvParam) {
 
 // ============== Interface functions ==============
 
-void ws2812_init(uint8_t u8Pin, uint32_t u32ApbClkFreq, Isr fTxEndCb, SWs2812FeederState *psFeederState) {
+void ws2812_init(uint8_t u8Pin, uint32_t u32ApbClkFreq, SWs2812State *psFeederState, Isr fTxEndCb, void *pvTxEndCbParam) {
   rmt_init_channel(psFeederState->sIface.eChannel, u8Pin, false);
   _rmt_config_channel(&psFeederState->sIface, u32ApbClkFreq / (1000 * RMT_FREQ_KHZ));
-  rmt_isr_register(psFeederState->sIface.eChannel, fTxEndCb, NULL, _feeder, NULL, psFeederState);
+  rmt_isr_register(psFeederState->sIface.eChannel, RMT_INT_TXTHRES, _feeder, psFeederState);
+  rmt_isr_register(psFeederState->sIface.eChannel, RMT_INT_TXEND, fTxEndCb, pvTxEndCbParam);
 }
 
-void ws2812_start(SWs2812FeederState *psFeederState) {
-    _feeder(psFeederState);
-    _feeder(psFeederState);
-    psFeederState->bBusy = true;
-    rmt_start_tx(psFeederState->sIface.eChannel, true);
+void ws2812_start(SWs2812State *psFeederState) {
+  psFeederState->szPos = 0;
+  _feeder(psFeederState);
+  _feeder(psFeederState);
+  rmt_start_tx(psFeederState->sIface.eChannel, true);
 }
 

modules/ws2812.h

@@ -21,17 +21,28 @@ extern "C" {
   } SWs2812Iface;
 
   typedef struct {
-    uint8_t *pu8Data;
-    size_t szLen;
-    size_t szPos;
-    bool bBusy;
-    SWs2812Iface sIface;
-  } SWs2812FeederState;
-
+    uint8_t *pu8Data;     ///< Points to data (GRB sequence) (Set by user)
+    size_t szLen;         ///< Length of the data (number of bytes)
+    size_t szPos;         ///< Current data cursor (Internal attribute)
+    SWs2812Iface sIface;  ///< Interface description
+  } SWs2812State;
+
+  // ============= Inline functions ===============
+  static inline SWs2812State ws2812_init_feederstate(uint8_t *pu8Data, size_t szLen, ERmtChannel eChannel, uint8_t u8Blocks) {
+    SWs2812Iface sIface = {
+      .eChannel = eChannel,
+      .u8Blocks = u8Blocks
+    };
+    return (SWs2812State){
+      .pu8Data = pu8Data,
+      .szLen = szLen,
+      .szPos = 0,
+      .sIface = sIface};
+  }
   // ============= Interface function declaration ===============
 
-  void ws2812_init(uint8_t u8Pin, uint32_t u32ApbClkFreq, Isr fTxEndCb, SWs2812FeederState *psFeederState);
-  void ws2812_start(SWs2812FeederState *psFeederState);
+  void ws2812_init(uint8_t u8Pin, uint32_t u32ApbClkFreq, SWs2812State *psFeederState, Isr fTxEndCb, void *pvTxEndCbParam);
+  void ws2812_start(SWs2812State *psFeederState);
 
 #ifdef __cplusplus
 }

src/rmt.c

@@ -16,7 +16,7 @@
 
 typedef struct {
   Isr aafIsr[8][4];
-  void *apvParam[8];
+  void *apvParam[8][4];
   uint8_t abChannelEn;
 } SRmtIntDispatcher;
 
@@ -37,7 +37,7 @@ IRAM_ATTR void _dispatch_isr(void *pvParam) {
         if (gpsRMT->arInt[RMT_INT_ST] & rmt_int_bit(iChannel, iInt)) {
           gpsRMT->arInt[RMT_INT_CLR] = rmt_int_bit(iChannel, iInt);
           if (NULL != psParam->aafIsr[iChannel][iInt]) {
-            psParam->aafIsr[iChannel][iInt](psParam->apvParam[iChannel]);
+            psParam->aafIsr[iChannel][iInt](psParam->apvParam[iChannel][iInt]);
           } else {
             // unhandled exception with registered RMT channel
           }
@@ -78,28 +78,18 @@ void rmt_isr_start(ECpu eCpu, uint8_t u8IntChannel) {
 /**
  * Registers ISRs to an RMT channel.
  * If a given interrupt type should not be handled, the corresponding Isr parameter should be NULL.
- * @param eChannel Identifies the RMT channel.
- * @param fTxEndIsr function to invoke in case of TXEND interrupt.
- * @param fRxEndIsr function to invoke in case of RXEND interrupt.
- * @param fTxThresholdIsr function to invoke in case of TXTHRESH interrupt.
- * @param fErrorIsr function to invoke in case of ERR interrupt.
+ * @param eChannel Identifies the RMT channel
+ * @param eIntType Interrupt type, either TXEND, RXEND, ERR or TXTHRES.
+ * @param fIsr Function to invoke in case of eIntType interrupt.
  * @param pvParam parameter passed to the Isr functions.
  */
-void rmt_isr_register(ERmtChannel eChannel, Isr fTxEndIsr, Isr fRxEndIsr, Isr fTxThresholdIsr, Isr fErrorIsr, void *pvParam) {
+void rmt_isr_register(ERmtChannel eChannel, ERmtIntType eIntType, Isr fIsr, void *pvParam) {
   gsIntDispatcher.abChannelEn |= (1 << eChannel);
 
-  gsIntDispatcher.aafIsr[eChannel][RMT_INT_TXEND] = fTxEndIsr;
-  gsIntDispatcher.aafIsr[eChannel][RMT_INT_RXEND] = fRxEndIsr;
-  gsIntDispatcher.aafIsr[eChannel][RMT_INT_TXTHRES] = fTxThresholdIsr;
-  gsIntDispatcher.aafIsr[eChannel][RMT_INT_ERR] = fErrorIsr;
+  gsIntDispatcher.aafIsr[eChannel][eIntType] = fIsr;
+  gsIntDispatcher.apvParam[eChannel][eIntType] = pvParam;
 
-  gsIntDispatcher.apvParam[eChannel] = pvParam;
-
-  Reg rIntMask = 0;
-  for (int iInt = 0; iInt < 4; ++iInt) {
-    if (NULL != gsIntDispatcher.aafIsr[eChannel][iInt]) rIntMask |= rmt_int_bit(eChannel, iInt);
-  }
-  gpsRMT->arInt[RMT_INT_ENA] |= rIntMask;
+  gpsRMT->arInt[RMT_INT_ENA] |= rmt_int_bit(eChannel, eIntType);
 }
 
 /**

src/rmt.h

@@ -254,7 +254,7 @@ extern "C" {
   void rmt_init_channel(ERmtChannel eChannel, uint8_t u8Pin, bool bInitLevel);
   void rmt_isr_init();
   void rmt_isr_start(ECpu eCpu, uint8_t u8IntChannel);
-  void rmt_isr_register(ERmtChannel eChannel, Isr fTxEndIsr, Isr fRxEndIsr, Isr fTxThresholdIsr, Isr fErrorIsr, void *pvParam);
+  void rmt_isr_register(ERmtChannel eChannel, ERmtIntType eIntType, Isr fIsr, void *pvParam);
 
 #ifdef __cplusplus
 }