Pre-compute syncword bytes

lib/Radio/MiLightRadioConfig.cpp

@@ -1,8 +1,8 @@
 #include <MiLightRadioConfig.h>
 
 MiLightRadioConfig MiLightRadioConfig::ALL_CONFIGS[] = {
-  MiLightRadioConfig(0x147A, 0x258B, 7, 9, 40, 71), // rgbw
-  MiLightRadioConfig(0x050A, 0x55AA, 7, 4, 39, 74), // cct
-  MiLightRadioConfig(0x7236, 0x1809, 9, 8, 39, 70), // rgb+cct, fut089
-  MiLightRadioConfig(0x9AAB, 0xBCCD, 6, 3, 38, 73)  // rgb
+  MiLightRadioConfig(0x147A, 0x258B, 7, 9, 40, 71, 0xAA, 0x05), // rgbw
+  MiLightRadioConfig(0x050A, 0x55AA, 7, 4, 39, 74, 0xAA, 0x05), // cct
+  MiLightRadioConfig(0x7236, 0x1809, 9, 8, 39, 70, 0xAA, 0x05), // rgb+cct, fut089
+  MiLightRadioConfig(0x9AAB, 0xBCCD, 6, 3, 38, 73, 0x55, 0x0A)  // rgb
 };

lib/Radio/MiLightRadioConfig.h

@@ -1,6 +1,7 @@
 #include <Arduino.h>
 #include <MiLightRemoteType.h>
 #include <Size.h>
+#include <RadioUtils.h>
 
 #ifndef _MILIGHT_RADIO_CONFIG
 #define _MILIGHT_RADIO_CONFIG
@@ -10,6 +11,7 @@
 class MiLightRadioConfig {
 public:
   static const size_t NUM_CHANNELS = 3;
+  static const uint8_t SYNCWORD_LENGTH = 5;
 
   MiLightRadioConfig(
     const uint16_t syncword0,
@@ -17,20 +19,37 @@ public:
     const size_t packetLength,
     const uint8_t channel0,
     const uint8_t channel1,
-    const uint8_t channel2
-  )
-    : syncword0(syncword0),
-      syncword3(syncword3),
-      packetLength(packetLength)
+    const uint8_t channel2,
+    const uint8_t preamble,
+    const uint8_t trailer
+  ) : syncword0(syncword0)
+    , syncword3(syncword3)
+    , packetLength(packetLength)
   {
     channels[0] = channel0;
     channels[1] = channel1;
     channels[2] = channel2;
+
+    size_t ix = SYNCWORD_LENGTH;
+
+    // precompute the syncword for the nRF24.  we include the fixed preamble and trailer in the
+    // syncword to avoid needing to bitshift packets.  trailer is 4 bits, so the actual syncword
+    // is no longer byte-aligned.
+    syncwordBytes[ --ix ] = reverseBits(
+      ((syncword0 << 4) & 0xF0) | (preamble & 0x0F)
+    );
+    syncwordBytes[ --ix ] = reverseBits((syncword0 >> 4) & 0xFF);
+    syncwordBytes[ --ix ] = reverseBits(((syncword0 >> 12) & 0x0F) + ((syncword3 << 4) & 0xF0));
+    syncwordBytes[ --ix ] = reverseBits((syncword3 >> 4) & 0xFF);
+    syncwordBytes[ --ix ] = reverseBits(
+      ((syncword3 >> 12) & 0x0F) | ((trailer << 4) & 0xF0)
+    );
   }
 
-  const uint16_t syncword0;
-  const uint16_t syncword3;
   uint8_t channels[3];
+  uint8_t syncwordBytes[SYNCWORD_LENGTH];
+  uint16_t syncword0, syncword3;
+
   const size_t packetLength;
 
   static const size_t NUM_CONFIGS = 4;

lib/Radio/NRF24MiLightRadio.cpp

@@ -35,8 +35,7 @@ int NRF24MiLightRadio::begin() {
 }
 
 int NRF24MiLightRadio::configure() {
-
-  retval = _pl1167.setSyncword(_config.syncword0, _config.syncword3);
+  int retval = _pl1167.setSyncword(_config.syncwordBytes, MiLightRadioConfig::SYNCWORD_LENGTH);
   if (retval < 0) {
     return retval;
   }

lib/Radio/PL1167_nRF24.cpp

@@ -11,6 +11,8 @@
  */
 
 #include "PL1167_nRF24.h"
+#include <RadioUtils.h>
+#include <MiLightRadioConfig.h>
 
 static uint16_t calc_crc(uint8_t *data, size_t data_length);
 static uint8_t reverse_bits(uint8_t data);
@@ -26,7 +28,7 @@ int PL1167_nRF24::open()
   _radio.setDataRate(RF24_1MBPS);
   _radio.disableCRC();
 
-  _syncwordLength = 5;
+  _syncwordLength = MiLightRadioConfig::SYNCWORD_LENGTH;
   _radio.setAddressWidth(_syncwordLength);
 
   return recalc_parameters();
@@ -37,39 +39,26 @@ int PL1167_nRF24::recalc_parameters()
   int packet_length = _maxPacketLength + 2;
   int nrf_address_pos = _syncwordLength;
 
-  if (_syncword0 & 0x01) {
-    _nrf_pipe[ --nrf_address_pos ] = reverse_bits( ( (_syncword0 << 4) & 0xf0 ) + 0x05 );
-  } else {
-    _nrf_pipe[ --nrf_address_pos ] = reverse_bits( ( (_syncword0 << 4) & 0xf0 ) + 0x0a );
+  if (packet_length > sizeof(_packet) || nrf_address_length < 3) {
+    return -1;
   }
-  _nrf_pipe[ --nrf_address_pos ] = reverse_bits( (_syncword0 >> 4) & 0xff);
-  _nrf_pipe[ --nrf_address_pos ] = reverse_bits( ( (_syncword0 >> 12) & 0x0f ) + ( (_syncword3 << 4) & 0xf0) );
-  _nrf_pipe[ --nrf_address_pos ] = reverse_bits( (_syncword3 >> 4) & 0xff);
-  _nrf_pipe[ --nrf_address_pos ] = reverse_bits( ( (_syncword3 >> 12) & 0x0f ) + 0x50 );	// kh: spi says trailer is always "5" ?
 
-  _receive_length = packet_length;
+  if (_syncwordBytes != nullptr) {
+    _radio.openWritingPipe(_syncwordBytes);
+    _radio.openReadingPipe(1, _syncwordBytes);
+  }
 
-  _radio.openWritingPipe(_nrf_pipe);
-  _radio.openReadingPipe(1, _nrf_pipe);
+  _receive_length = packet_length;
 
   _radio.setChannel(2 + _channel);
-
   _radio.setPayloadSize( packet_length );
 
   return 0;
 }
 
-int PL1167_nRF24::setTrailerLength(uint8_t trailerLength)
-{ return 0; }
-/* kh- no thanks, I'll take care of that.
-   One could argue there is potential value to "defining" the trailer - such that
-   we can use those "values" for internal (repeateR?) functions since they are
-   ignored by the real PL1167..  But there is no value in _this_ implementation...
-*/
-
-int PL1167_nRF24::setCRC(bool crc)
-{
-  _crc = crc;
+int PL1167_nRF24::setSyncword(const uint8_t syncword[], size_t syncwordLength) {
+  _syncwordLength = syncwordLength;
+  _syncwordBytes = syncword;
   return recalc_parameters();
 }