sandeepmistry / arduino-LoRa

An Arduino library for sending and receiving data using LoRa radios.
MIT License
1.6k stars 621 forks source link

Force LDO option set or unset #668

Closed zbx-sadman closed 8 months ago

zbx-sadman commented 9 months ago

Some remote clients needs LDO using on any Spreading Factor. CDEbyte E32 (and similar) modules for example.

zbx-sadman commented 9 months ago

Hello @sandeepmistry

Library author known better how to make his code pretty and readable, i guess.

Honestly, i modified source a bit deeper and it can be written now as follows:

LoRa.h

class LoRaClass : public Stream {
public:
  LoRaClass();
...
  void setLdoFlagForced(const boolean);  // can be replaced with enableLowDataRateOptimize() & disableLowDataRateOptimize()
  void enableLowDataRateOptimize();
  void disableLowDataRateOptimize();
...
private:
...
  boolean _ldoForced;
...
};

LoRa.cpp

void LoRaClass::setLdoFlag()
{
  boolean ldoOn = true;

  if (!_ldoForced) {
     // Section 4.1.1.5
     long symbolDuration = 1000 / ( getSignalBandwidth() / (1L << getSpreadingFactor()) ) ;

     // Section 4.1.1.6
     ldoOn = symbolDuration > 16;
  }

  uint8_t config3 = readRegister(REG_MODEM_CONFIG_3);
  bitWrite(config3, 3, ldoOn);
  writeRegister(REG_MODEM_CONFIG_3, config3);
}

// can be replaced with enableLowDataRateOptimize() & disableLowDataRateOptimize()
void LoRaClass::setLdoFlagForced(const boolean ldoForced)
{
  _ldoForced = ldoForced; 
  setLdoFlag();
}

void LoRaClass::enableLowDataRateOptimize() 
{
  _ldoForced = true; 
  setLdoFlag();
}

void LoRaClass::disableLowDataRateOptimize() 
{
  _ldoForced = false; 
  setLdoFlag();
}

Let me know if i must create another pull-request.

Thanks for your helpful library.

sandeepmistry commented 9 months ago

Hi @zbx-sadman,

Thanks for considering the feedback, I would suggest making the void setLdoFlagForced(const boolean); private, and removing _ldoForced. Other than your comments in https://github.com/sandeepmistry/arduino-LoRa/pull/668#issuecomment-1742557157 look good. Feel free to make edits to this pull request or make a new one.

zbx-sadman commented 8 months ago

Hello @sandeepmistry, i've add separated setter methods for LDO flag. This is right way for library?

sandeepmistry commented 8 months ago

@zbx-sadman yes, that looks good. I've pushed some minor changes in https://github.com/sandeepmistry/arduino-LoRa/pull/668/commits/1928aabe48fa8da785820f079a901b4cfa6e8601 to update the keywords.txt and move the private function declaration in the .h earlier in the file.