[LR11XX] Add missing override specifiers

[lyusupov]

Build of an MCVE sketch like that:

#include <RadioLib.h>

Module *mod;
LR11x0  *radio_semtech;

void setup()
{
    int probed_radio = 1;

    mod = new Module(RADIOLIB_NC, RADIOLIB_NC, RADIOLIB_NC, RADIOLIB_NC);

    switch (probed_radio)
    {
      case 0:
        radio_semtech = new LR1110(mod);
        break;

      case 1:
      default:
        radio_semtech = new LR1121(mod);
        break;
    }

    radio_semtech->begin(125.0, 9, 7, RADIOLIB_LR11X0_LORA_SYNC_WORD_PRIVATE, 8, 1.6);
    radio_semtech->setOutputPower(0, true);
}

void loop()
{

}

causes a failure:

/tmp/7/hp/hp.ino: In function 'void setup()':
hp:27:42: error: no matching function for call to 'LR11x0::setOutputPower(int, bool)'
     radio_semtech->setOutputPower(0, true);
                                          ^
In file included from /home/user/Arduino/libraries/RadioLib/src/modules/CC1101/CC1101.h:7,
                 from /home/user/Arduino/libraries/RadioLib/src/RadioLib.h:69,
                 from /tmp/7/hp/hp.ino:1:
/home/user/Arduino/libraries/RadioLib/src/modules/CC1101/../../protocols/PhysicalLayer/PhysicalLayer.h:374:21: note: candidate: 'virtual int16_t PhysicalLayer::setOutputPower(int8_t)'
     virtual int16_t setOutputPower(int8_t power);
                     ^~~~~~~~~~~~~~
/home/user/Arduino/libraries/RadioLib/src/modules/CC1101/../../protocols/PhysicalLayer/PhysicalLayer.h:374:21: note:   candidate expects 1 argument, 2 provided

This PR is intended to fix the issue.

[jgromes]

Hmm, this does not seem completely correct - why should PhysicalLayer need to have the LR11x0-specific virtual member setOutputPower(int8_t power, bool forceHighPower)? It doesn't have any other module-specific members (like SX1272::setOutputPower(int8_t power, bool useRfo) - that's why cppcheck started complaining.

I think the real issue is stated in the original error message: error: no matching function for call to 'LR11x0::setOutputPower(int, bool)'. There is indeed no such method, and that's because your radio_semtech object still has the type LR11x0*, not LR1110* or LR1121* as you might think.

[lyusupov]

I need to use the LR11xx object pointer in order to support both LR1110 and LR1121 ICs within the same code:

https://github.com/lyusupov/SoftRF/blob/078b8fdd25c17cbd387fbe60db2b9ab1cdd96c59/software/firmware/source/SoftRF/src/driver/radio/radiolib.cpp#L455-L832

Every 'overrides' of LR11XX class such as

• setFrequency
• setOutputPower ( int8_t pwr)

are confirmed to work just fine with real LR1110 (Seed T1000-E) and LR1121 (Ebyte E80) hardware

But I've got an issue with HPDTeK HPD-16E module (LR1121) - they do not use TX_LP pin, only TX_HP is connected to RF switch and antenna circuits.
That's why I need to use setOutputPower() method with two arguments.

I've actually verified this PR on the real hardware that I have - it works fine to me on both LR1110 and LR1121.

[jgromes]

I've actually verified this PR on the real hardware that I have - it works fine to me on both LR1110 and LR1121.

That's not the issue, this PR is based on a faulty understanding of how inheritance works. Just because you used LR1110 constructor to instantiate the radio_semtech object, it does not make it change its type from LR11x0*. Think of it like this:

float f = 5.0;
int i = f;

Does this mean that after the second assignment, the type of the variable i changes? No, it does not, and that is also the case in your code. Feel free to verify that with a debugger. What you did is you add a new setOutputPower virtual member to PhysicalLayer, and is what's getting used.

Bottom line is this is not the correct way to do this. Either you have to use the PhysicalLayer interface as shown in the PhysicalLayer example, or, if you have something specific to a given class, you need to cast it as such and then call that specific method, something like this (not tested with hardware, but does compile). Of course, have to make sure that the module-specific methods will only be called if the radio object has actually been allcoated by that module's contructor!

  // some custom radio selection logic
  bool lr1110;
  #if RADIO_LR1110 
  LR1110 radio = new Module(10, 2, 9, 3);
  lr1110 = true;
  #else
  SX1261 radio = new Module(10, 2, 9, 3);
  lr1110 = false;
  #endif
  
  PhysicalLayer* phy = (PhysicalLayer*)&radio;

  // this is common to the PHY interface
  uint8_t* data;
  size_t len;
  phy->transmit(data, len);

  // now I need to do something module-specific
  if(lr1110) {
    LR1110* ptr = (LR1110*)phy;
    ptr->setOutputPower(10, true);
  }

[lyusupov]

@jgromes

No, it does not, and that is also the case in your code. Feel free to verify that with a debugger. What you did is you add a new setOutputPower virtual member to PhysicalLayer, and is what's getting used.

For sure I did that before applying this PR.

When I changed the true on false in the code below

then a radio receiver located nearby stops receiving any packets because signal level is too low (LP_TX is NC).

If virtual setOutputPower() method defined in PhysicalLayer would work - there would be no difference of the radio operation. The radio TX power setting would stay on the IC default level in both cases.

[jgromes]

The core issue with your code is that setOutputPower(int8_t power, bool forceHighPower) only makes sense for LR11x0 radios - so why should that be in the PhysicalLayer? That class is designed to only contain the methods that are common to all the radio modules. Because of that, this cannot be merged.

[lyusupov]

I will be happy to use your own PR that will give the same effect necessary. One of the options I can see is to supply an extra argument for LR11XX begin() method to pass this 'forceHighPower' requirement