Skip to content

sx127x 20dBm support #79

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

sx127x 20dBm support #79

wants to merge 1 commit into from

Conversation

dontsovcmc
Copy link
Contributor

#77

@dontsovcmc dontsovcmc mentioned this pull request Feb 27, 2018
@bkeevil
Copy link

bkeevil commented Apr 8, 2018

Looks good to me

@wero1414
Copy link
Collaborator

I do not see anything wrong with this improvement, but i would suggest to not duplicate functions, cause this library contains already one function to change the power during Tx so, i would suggest to add the function set20dBm_sx127x() as an option inside the setTxPower() function before its if statement about the PA_OUTPUT_RFO_PIN (

void LoRaClass::setTxPower(int level, int outputPin)
)

@dontsovcmc
Copy link
Contributor Author

Good idea!
Also, I think it's better call setOCP_sx127x() (current limit) before writeRegister(REG_PA_DAC, ..).

@dontsovcmc
Copy link
Contributor Author

The question:
should we change OCP inside setTxPower function, or we should add notice for user "you must call setOCP(140); before setTxPower(20) ?

void LoRaClass::setTxPower(int level, int outputPin)
{
  if (PA_OUTPUT_RFO_PIN == outputPin) {
    // RFO
    if (level < 0) {
      level = 0;
    } else if (level > 14) {
      level = 14;
    }

    writeRegister(REG_PA_CONFIG, 0x70 | level);
  } else {
    
    if (level > 17) {
      writeRegister(REG_PA_DAC, 0x87); //Value for High Power
      setOCP(140);
    } else {
      writeRegister(REG_PA_DAC, 0x84); //Default value PA_HF/LF or +17dBm
      setOCP(100);
    }
    
    // PA BOOST
    if (level < 2) {
      level = 2;
    } else if (level > 17) {
      level = 17;
    }

    writeRegister(REG_PA_CONFIG, PA_BOOST | (level - 2));
  }
}

@halukmy
Copy link

halukmy commented Apr 18, 2018

currently not supports 20dbm?

@wero1414
Copy link
Collaborator

Currently this library allows you only to transmit with a maximun of 17 dBm

@wero1414
Copy link
Collaborator

wero1414 commented May 2, 2018

@dontsovcmc i think that the better way is to change inside the OCP directly

@dontsovcmc
Copy link
Contributor Author

@sandeepmistry, what are you think about this improvments?

  1. should we call setOCP() inside setTxPower()?

remark: OCP - current protection limit.

@sandeepmistry
Copy link
Owner

@dontsovcmc here is my thoughts on what I'd like to see:

  • no changes to the current public facing API's
  • use the current public facing LoRa.setTxPower(int level, int outputPin)
    • if the outputPin is set to PA_OUTPUT_RFO_PIN: leave the current behaviour
    • if the outputPin is set to PA_OUTPUT_PA_BOOST_PIN :
      • if level is between 2 to 17: use the current behaviour, and also set OCP and PA DAC accordingly
      • if level is between 18 to 20: support these new ones by setting OCP, PA DAC and PA CONFIG accordingly.

@dontsovcmc @wero1414 any thoughts on the above?

@wero1414
Copy link
Collaborator

wero1414 commented May 8, 2018

Agree if you filter in agreement the level of transmission power you can set the correct configuration to control de current, for me its a real good improvement.

@sandeepmistry
Copy link
Owner

Closing in favour of #153.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants