Skip to content
New issue

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

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

Already on GitHub? # to your account

LMIC_setDrTxpow() is ineffective #21

Closed
terrillmoore opened this issue Jul 14, 2017 · 11 comments
Closed

LMIC_setDrTxpow() is ineffective #21

terrillmoore opened this issue Jul 14, 2017 · 11 comments
Assignees
Labels

Comments

@terrillmoore
Copy link
Member

terrillmoore commented Jul 14, 2017

LMIC_setDrTxpow() doesn't really do anything.

Although calling it sets LMIC.adrTxpow to the input parameter, the value is then never used inside the LMIC library. This is because LMIC's radio.c uses LMIC.txpow, and in US915, lmic.c::updatetx() sets LMIC.txpow to 30 for 125kHz channels, and 26 for 500kHz channels, ignoring LMIC.adrTxpow. Finally, radio.c configPower() limits the value to 17 dBm, and doesn't turn on the +20 dBm option if over 17 dBm. This wouldn't cause much of a problem in Europe, as you're limited to 16dBm EIRP, and so we have to subtract 3 dBm anyway. But in the US it's a limit.

Exact rules in the US (per LoRaWAN 1.0.2)

  • 30 dBm Tx power is allowed if you're using more than 50 channels.
  • 26 dBm Tx power is allowed if you're using 500 kHz bandwidth.
  • 21 dBm Tx power is allowed if you're using fewer than 50 channels at 125 kHz.

At present, however, it seems that neither manual nor automatic TX power control can set the power above 10 dBm; and (in the US), the TX power is fixed at 10 dBm.

To fix this:

  • fix the handling of > 10 dBm in radio.c
  • fix the US updatetx() so that ADR works
  • test ADR in US, EU, etc.
@terrillmoore
Copy link
Member Author

See #50 #46 etc.

@terrillmoore terrillmoore self-assigned this Feb 7, 2018
@ngraziano
Copy link

For EU like region we can do something like this commit ngraziano/arduino-lmic@3948c8c

It will allow the network to reduce power if quality of signal is good enough (TTN do that).

@terrillmoore
Copy link
Member Author

terrillmoore commented Nov 28, 2018

@ngraziano Thanks for the reference, that make it clear.

I see that. in fact, this management of txpow is one of the important things that *_updateTx() is supposed to do.

I like the idea of using the ADR value as a maximum. But after reviewing the code, I think it might be better to put it more centrally.

It turns out that txpow is only meaningfully referenced one place in radio.c around line 445. Allowing for some refactoring, that's where we should consider adrTxPow. Here's the current code:

static void configPower () {
#ifdef CFG_sx1276_radio
// PA_BOOST output is assumed but not 20 dBm.
s1_t pw = (s1_t)LMIC.txpow;
if(pw > 17) {
pw = 17;
} else if(pw < 2) {
pw = 2;
}
// 0x80 forces use of PA_BOOST; but we don't
// turn on 20 dBm mode. So powers are:
// 0000 => 2dBm, 0001 => 3dBm, ... 1111 => 17dBm
// But we also enforce that the high-power mode
// is off by writing RegPaDac.
writeReg(RegPaConfig, (u1_t)(0x80|(pw - 2)));
writeReg(RegPaDac, readReg(RegPaDac)|0x4);
#elif CFG_sx1272_radio
// set PA config (2-17 dBm using PA_BOOST)
s1_t pw = (s1_t)LMIC.txpow;
if(pw > 17) {
pw = 17;
} else if(pw < 2) {
pw = 2;
}
writeReg(RegPaConfig, (u1_t)(0x80|(pw-2)));
#else
#error Missing CFG_sx1272_radio/CFG_sx1276_radio
#endif /* CFG_sx1272_radio */
}

Instead ("NOTE" comments added to explain why I changed things, other comments intended for publication)

static void configPower () {
    // NOTE: no need for (s1_t) cast, txpow is declared s1_t.
    s1_t pw = LMIC.txpow;

   // enforce ADR limitations and adminstrative power limits here. The
   // bandplans set txpow to the desired target in terms of power at
   // the radio. 
    if (pw > LMIC.adrTxPow)
        pw = LMIC.adrTxPow;

   // TODO(tmm@mcci.com) this also is where we'd adjust for antenna gain other than 3 dBi.

#ifdef CFG_sx1276_radio
    // TODO(tmm@mcci.com) now that we have the enhanced HAL, we could delegate this work,
    // thereby allowing PAs to work properly on devices like the Murata that support 20 dB tx.
    // In fringes, an extra 3 dB could be very helpful.

    // PA_BOOST output is assumed but not 20 dBm.
    if(pw > 17) {
        pw = 17;
    } else if(pw < 2) {
        pw = 2;
    }
    // 0x80 forces use of PA_BOOST; but we don't 
    //    turn on 20 dBm mode. So powers are:
    //    0000 => 2dBm, 0001 => 3dBm, ... 1111 => 17dBm
    // But we also enforce that the high-power mode
    //    is off by writing RegPaDac.
    writeReg(RegPaConfig, (u1_t)(0x80|(pw - 2)));
    writeReg(RegPaDac, readReg(RegPaDac)|0x4);

#elif CFG_sx1272_radio
    // set PA config (2-17 dBm using PA_BOOST)
    if(pw > 17) {
        pw = 17;
    } else if(pw < 2) {
        pw = 2;
    }
    writeReg(RegPaConfig, (u1_t)(0x80|(pw-2)));
#else
#error Missing CFG_sx1272_radio/CFG_sx1276_radio
#endif /* CFG_sx1272_radio */
}

I need some volunteers to test... or I need to get this working with the RWC5020A. But that is a few weeks off.

@ngraziano
Copy link

I check regional parameter and I see a problem with region like AS923 where Max EIRP is set by TxParamSetupReq.
In actual code for AS923, adrTxPow store the delta and not the absolute value.

Maybe adrTxPow should store the delta for all region and not the absolute value.

But I am little lost because in lorawan regional parameters for E868 do not match the code and there is no power per band.

This topic need more thinking.

@pomplesiegel
Copy link

pomplesiegel commented Jan 26, 2019

Any thoughts on how this would be resolved? Any quick hard-coded fixes for those of us using this only one region and trying to LIMIT power? In my case, I would like to put the RFM95W to its lowest power level (in USA at 915Mhz), but calls like

LMIC_setDrTxpow(DR_SF7,2);

and

LMIC.datarate = DR_SF7;
LMIC.txpow = 2; 

seemingly still have no effect.

Thank you!

@pomplesiegel
Copy link

Update: I'm just manually setting

s1_t pw = 2; 

in radio.c, as my region and location will be static. Still though, is there a plan to resolve
LMIC_setDrTxpow() and restore its effectiveness? Am I correct in believing that it does not affect the Spread factor as well?

Thank you!

@terrillmoore
Copy link
Member Author

@pomplesiegel -- LMIC_setDrTxpow() sets the spreading factor, but that will get reset by network downlinks that happen during and after a joint. In other words, you may find that you have to set the data rate several times to make things effective, and you may have to turn off ADR (which can be done, and which works). Network operators are not always happy if you do this, but... sometimes it's necessary.

As for changing the tx power, yes... the delay is due to lack of time on my part to set up a proper test -- we really want to make sure we're controlling power properly.

I have been planning to run US LoRaWAN certification tests, which would (I think) clear up enough of the otehr issues to let this be addressed. But I don't know if I'll get to it this quarter, because of pressing work for MCCI's other projects.

@pomplesiegel
Copy link

@terrillmoore, thanks so much for the in-depth response. This is very helpful information on the spreading factor, as I had heard varying reports from others.

I totally understand about the tx power issue and the need to get it right. In the meantime people like me will just hard-code a value and that's fine.

Very exciting about the US LoraWan cert tests. I imagine that is a major question on many of our minds: When following best practices and using sanctioned hardware with this fork of LMIC, are we being compliant and future-proof with our devices?

Thanks again!

@terrillmoore
Copy link
Member Author

This is now fixed with #416 and #410

@Commifreak
Copy link

I have an additional question about the Tx power... What is the initial value if nothing is set? I use the lib for an SX1276 with this config:

#define CFG_eu868
#define CFG_sx1276_radio 1
#define DISABLE_PING
#define DISABLE_BEACONS

#define LMIC_DEBUG_LEVEL 2

Thanks :)

@martin3000
Copy link

see lorabase_eu868.h :
EU868_TX_EIRP_MAX_DBM = 16

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

No branches or pull requests

5 participants