arduino-LoRa icon indicating copy to clipboard operation
arduino-LoRa copied to clipboard

LowDataRateOptimize cleared in setGain

Open engeen-nl opened this issue 4 years ago • 6 comments

I noticed the LowDataRateOptimize bit of REG_MODEM_CONFIG_3 is cleared in setGain. (LoRa.cpp:637)

Shouldn't setLdoFlag() be called after?

engeen-nl avatar Jul 30 '21 11:07 engeen-nl

You mean these codes? image

Last time, I ported over from RadioLib: https://github.com/jgromes/RadioLib/blob/master/src/modules/SX127x/SX1278.cpp#L334

image

image

Why should we set LowDataRateOptimize bit? Can you help to put in some light? Thanks a lot.

IoTThinks avatar Aug 02 '21 03:08 IoTThinks

In the RadioLib code only bit 2 is set to 0. (I hope that's what SPIsetRegValue does). By calling: writeRegister(REG_MODEM_CONFIG_3, 0x00); You're clearing all bits, while it could be that the LowDataRateOptimize was set earlier by setLdoFlag(). (Interesting to see you ported from radiolib. arduino-Lora is really clean, good job!)

engeen-nl avatar Aug 02 '21 10:08 engeen-nl

Ok, let me do a fix :D Any suggestion? A bit blurred with work.

IoTThinks avatar Aug 02 '21 10:08 IoTThinks

Here's where I should do a pull request I guess, but I also have to test it later. For now:

void LoRaFSKClass::setGain(uint8_t gain)
{
  // check allowed range
  if (gain > 6) {
    gain = 6;
  }
  
  // set to standby
  idle();
  
  uint8_t config3 = readRegister(REG_MODEM_CONFIG_3);
  bitWrite(config3, 2, gain);
  writeRegister(REG_MODEM_CONFIG_3, config3);

  // set gain
  if (gain == 0) {
    // if gain = 0, enable AGC
    //writeRegister(REG_MODEM_CONFIG_3, 0x04);
  } else {
    // disable AGC
    //writeRegister(REG_MODEM_CONFIG_3, 0x00);
	
    // clear Gain and set LNA boost
    writeRegister(REG_LNA, 0x03);
	
    // set gain
    writeRegister(REG_LNA, readRegister(REG_LNA) | (gain << 5));
  }
}

engeen-nl avatar Aug 02 '21 13:08 engeen-nl

Or add another function to toggle a bit: LoRa.h:

  void setRegisterBit(uint8_t reg, uint8_t bit, uint8_t set = 1);
  void clearRegisterBit(uint8_t reg, uint8_t bit) { return setRegisterBit(reg, bit, 0); }

LoRa.cpp:

void LoRaFSKClass::setRegisterBit(uint8_t reg, uint8_t bit, uint8_t set /*= 1*/) {
  uint8_t val = readRegister(reg);
  bitWrite(val, bit, set);
  writeRegister(reg, val);
}

Then you can change all the other places with bitWrite with setRegisterBit and remove the read and write registers around it.

engeen-nl avatar Aug 02 '21 14:08 engeen-nl

I will try to fix this issue this week. I may use bit masking instead to fit the style of this library.

IoTThinks avatar Aug 09 '21 03:08 IoTThinks