Comment and error checking cleanup.

Fixed power adjustment for SX1261

src/modules/SX1261.cpp

@@ -1,6 +1,10 @@
 #include "SX1261.h"
 
-// note that this is untested (lacking the hardware) and based purely on the datasheet.
+SX1261::SX1261(Module* mod) 
+  : SX1262(mod) {
+
+}
+
 int16_t SX1261::setOutputPower(int8_t power) {  
   // check allowed power range
   if (!((power >= -17) && (power <= 14))) {
@@ -15,21 +19,19 @@ int16_t SX1261::setOutputPower(int8_t power) {
   }
 
   state = setOptimalLowPowerPaConfig(&power);
+  if (state != ERR_NONE) {
+    return(state);
+  }
 
   // set output power
   // TODO power ramp time configuration
-  if (state == ERR_NONE) {
+  state = SX126x::setTxParams(power);
-    state = SX126x::setTxParams(power);
+  if (state != ERR_NONE) {
+    return(state);
   }
 
   // restore OCP configuration
-  int16_t state2 = writeRegister(SX126X_REG_OCP_CONFIGURATION, &ocp, 1);
+  return writeRegister(SX126X_REG_OCP_CONFIGURATION, &ocp, 1);
-
-  if (state != ERR_NONE) {
-    return state;
-  } else {
-    return state2;
-  }
 }
 
 int16_t SX1261::setOptimalLowPowerPaConfig(int8_t* inOutPower)
@@ -40,7 +42,9 @@ int16_t SX1261::setOptimalLowPowerPaConfig(int8_t* inOutPower)
   }
   else {
     state = SX126x::setPaConfig(0x01, SX126X_PA_CONFIG_SX1261, 0x00);
-    *inOutPower -= 4;
+    // changing the PaConfig means output power is now scaled so we get 3 dB less than requested.
+    // see datasheet table 13-21 and comments in setOptimalHiPowerPaConfig.
+    *inOutPower -= 3;
   }
   return state;
 }

src/modules/SX1261.h

@@ -6,6 +6,8 @@
 #include "SX126x.h"
 #include "SX1262.h"
 
+//SX126X_CMD_SET_PA_CONFIG
+#define SX126X_PA_CONFIG_SX1261                       0x01
 
 // TODO: implement SX1261 class
 class SX1261 : public SX1262 {
@@ -15,10 +17,15 @@ class SX1261 : public SX1262 {
 
       \param mod Instance of Module that will be used to communicate with the radio.
     */
-    SX1261(Module* mod)
+    SX1261(Module* mod);
-      : SX1262(mod) {
-    }
 
+    /*!
+      \brief Sets output power. Allowed values are in range from -17 to 14 dBm.
+
+      \param power Output power to be set in dBm.
+
+      \returns \ref status_codes
+    */
     int16_t setOutputPower(int8_t power);
 
   private:

src/modules/SX1262.cpp

@@ -85,7 +85,7 @@ int16_t SX1262::setFrequency(float freq, bool calibrate) {
 
 int16_t SX1262::setOutputPower(int8_t power) {
   // check allowed power range
-  if (!(power >= -17 && power <= 22)) {
+  if (!((power >= -17) && (power <= 22))) {
     return(ERR_INVALID_OUTPUT_POWER);
   }
 
@@ -97,21 +97,20 @@ int16_t SX1262::setOutputPower(int8_t power) {
   }
 
   // this function sets the optimal PA settings 
-  // and scales our requested power based
+  // and adjusts power based on the PA settings chosen
+  // so that output power matches requested power.
   state = SX126x::setOptimalHiPowerPaConfig(&power);
+  if (state != ERR_NONE) {
+    return(state);
+  }
 
   // set output power
   // TODO power ramp time configuration
-  if (state == ERR_NONE) {
+  state = SX126x::setTxParams(power);
-    state = SX126x::setTxParams(power);
+  if (state != ERR_NONE) {
+    return(state);
   }
 
   // restore OCP configuration
-  int16_t state2 = writeRegister(SX126X_REG_OCP_CONFIGURATION, &ocp, 1);
+  return writeRegister(SX126X_REG_OCP_CONFIGURATION, &ocp, 1);
-  
-  if (state != ERR_NONE) {
-    return state;
-  } else {
-    return state2;
-  }
 }

src/modules/SX1268.cpp

@@ -78,9 +78,6 @@ int16_t SX1268::setFrequency(float freq, bool calibrate) {
 
 int16_t SX1268::setOutputPower(int8_t power) {
   // check allowed power range
-  // TODO with optimal PA config
-  // it's likely possible this could be expanded to go down to -17 just like for SX1262.
-  // but the datasheet doesn't explicitly state that and I don't have SX1268 unit to test with
   if(!((power >= -9) && (power <= 22))) {
     return(ERR_INVALID_OUTPUT_POWER);
   }
@@ -92,21 +89,19 @@ int16_t SX1268::setOutputPower(int8_t power) {
     return(state);
   }
 
-  // enable optimal PA
+  // enable optimal PA - this changes the value of power.
   state = SX126x::setOptimalHiPowerPaConfig(&power);
+  if (state != ERR_NONE) {
+    return(state);
+  }
 
   // set output power
   // TODO power ramp time configuration
-  if (state == ERR_NONE) {
+  state = SX126x::setTxParams(power);
-    state = SX126x::setTxParams(power);
+  if (state != ERR_NONE) {
+    return(state);
   }
 
   // restore OCP configuration
-  int16_t state2 = writeRegister(SX126X_REG_OCP_CONFIGURATION, &ocp, 1);
+  return writeRegister(SX126X_REG_OCP_CONFIGURATION, &ocp, 1);
-
-  if (state != ERR_NONE) {
-    return state;
-  } else {
-    return state2;
-  }
 }

src/modules/SX126x.cpp

@@ -1127,29 +1127,29 @@ int16_t SX126x::setTxParams(uint8_t power, uint8_t rampTime) {
   return(SPIwriteCommand(SX126X_CMD_SET_TX_PARAMS, data, 2));
 }
 
-// set PA config for optimal consumption as described in section 13-21 of the datasheet:
+// set PA config for optimal consumption as described in section 13-21 of the datasheet.
-// the final column of Table 13-21 suggests that the value passed in SetTxParams 
+int16_t SX126x::setOptimalHiPowerPaConfig(int8_t * inOutPower)
-// is actually scaled depending on the parameters of setPaConfig.
-// Testing confirms this is approximately right
-int16_t SX126x::setOptimalHiPowerPaConfig(int8_t* inOutPower)
 {
+  // the final column of Table 13-21 suggests that the value passed in SetTxParams 
+  // is actually scaled depending on the parameters of setPaConfig.
+  // Testing confirms this is approximately right
   int16_t state;
   if (*inOutPower >= 21) {
     state = SX126x::setPaConfig(0x04, SX126X_PA_CONFIG_SX1262_8, SX126X_PA_CONFIG_HP_MAX/*0x07*/);
   }
   else if (*inOutPower >= 18) {
     state = SX126x::setPaConfig(0x03, SX126X_PA_CONFIG_SX1262_8, 0x05);
+    // datasheet instructs request 22 dBm for 20 dBm actual output power
     *inOutPower += 2;
-  }
+  } else if (*inOutPower >= 15) {
-  else if (*inOutPower >= 15) {
     state = SX126x::setPaConfig(0x02, SX126X_PA_CONFIG_SX1262_8, 0x03);
+    // datasheet instructs request 22 dBm for 17 dBm actual output power
     *inOutPower += 5;
-  }
+  } else {
-  else {
     state = SX126x::setPaConfig(0x02, SX126X_PA_CONFIG_SX1262_8, 0x02);
+    // datasheet instructs request 22 dBm for 14 dBm actual output power.
     *inOutPower += 8;
   }
-  // TODO investigate if better power efficiency can be achieved using undocumented even lower settings
   return state;
 }
 

src/modules/SX126x.h

@@ -152,7 +152,6 @@
 //SX126X_CMD_SET_PA_CONFIG
 #define SX126X_PA_CONFIG_HP_MAX                       0x07
 #define SX126X_PA_CONFIG_PA_LUT                       0x01
-#define SX126X_PA_CONFIG_SX1261                       0x01
 #define SX126X_PA_CONFIG_SX1262_8                     0x00
 
 //SX126X_CMD_SET_RX_TX_FALLBACK_MODE