LoRaWAN: band at compile time, bands in general.

[ropg]

1

I noticed that all the examples use a global instance of LoRaWANNode. The constructor requires the band and subBand to be supplied, and short of God Mode there is no way to change that later. What that means is that you can have a global instance only if you know the band and subBand at compile time. So any device or not-user-compiled thing must either be compiled for a given band, or use a pointer and new LoRaWANNode (which is what I did in my persistence thing). This isn't a problem per-se, but it's something to be aware of as a design choice. It means anyone that makes a device and wants to interact with a node instance must use pointers and -> everywhere.

If it were to be decided that that's a problem, one could envision an overloaded constructor without arguments, a setBand() and some nullptr checks and possibly a status code to see if a band has been set. I could put that in, but we could also leave as is.

2

Whether through the use of pointers or with setBand, there will be code that selects bands. Be it in a dropdown on a web interface, in an app with Bluetooth. Right now there's no mechanism, other than looking at the source and duplicating the list, to see what bands are supported or to convert between some storable version and the compilation-dependent pointer that is needed for LoRaWANNode. It would be fairly straightforward to have a few functions to convert between a textual char* with the band name and the pointer, to tell if a band name is valid and to enumerate valid bands.

This now lives in my ESP32 persistence thing, but that's (IMHO) probably not where it belongs. Would be happy to provide a PR, but again only if people think it's a good idea.

[jgromes]

The constructor requires the band and subBand to be supplied

That is a conscious design choice. Knowing the band you operate on is essential for any operation in LoRaWAN, that's why you need to select one when creating the LoRaWAN object. I don't think LoRaWAN has any mechanism to dynamically switch band as the underlying assumption seems to be that nodes are not moving.

If you cannot determine the band time at compile time, then what you did is the valid approach: to delay instantiation of LoRaWAN object into runtime after the band is known (e.g. by the user selecting it). This avoids having any sort of "grey zone" when the LoRaWAN object does exist, but cannot be used in any way.

Right now there's no mechanism, other than looking at the source and duplicating the list, to see what bands are supported or to convert between some storable version and the compilation-dependent pointer that is needed for LoRaWANNode

Let's split this into two points - the first one is missing list of supported bands, which is an issue of incomplete documentation. The other point is valid, there is currenty no user-friendly way to programatically select the band. Seeing as there already is an enum LoRaWANBandNum_t, I think the cleanest solution would be to provide an additional constructor e.g.: LoRaWANNode::LoRaWANNode(PhysicalLayer* phy, LoRaWANBandNum_t band, uint8_t subBand). I'd very much like to avoid having to mess with string comparisons just to select from a very limited number of options ;)

[ropg]

To be clear: Heavens forbid I'm not talking about switching bands dynamically in the same LoRaWANNode instance, I'm talking about not having to select one at compile time. (Although an inter-band roaming protocol would be funny to submit. "But I want my session to continue when I get off the plane!"). It'd just be nice to have a global instance and only put in the band after you've read your flash. The theoretical setBand could be made to return 'INVALID_BAND' if there's already anything other than 'nullptr' there.

I'd very much like to avoid having to mess with string comparisons just to select from a very limited number of options ;)

Imagine a device with a web interface and a dropdown. It needs to show textual representation. It would be cool if they upgrade their RadioLib and the dropdown shows the supported bands. The maker of that web interface either needs to be able to get a list of band names from our code, or do manual work every time we add/delete bands.

[jgromes]

Heavens forbid I'm not talking about switching bands dynamically in the same LoRaWANNode instance.

I know you were not, but it has that effect, unless you explicitly limit the proposed setBand method to be only able to act once (which was not included in your original post). I remain unconvinced we should expose this mechanism. IMO you should be able to create the object only once you know the band.

Imagine a device with a web interface and a dropdown. It needs to show textual representation.

Those layers really should be separated, I don't see why RadioLib should provide its supported methods or LoRaWAN bands in textual representation - most people will never use that. I think that nearly all users will know the band they want to use at compile time. I also agree that there might be those who would find a programmatic way to specify the band more useful. For the latter, I propose what I think is the easiest way to achieve this.

EDIT: If it makes your life easier, we can have a BandLast element in the enum LoRaWANBandNum_t so that you can always tell how many bands are supported.

[ropg]

If it makes your life easier, we can have a BandLast element in the enum LoRaWANBandNum_t so that you can always tell how many bands are supported.

• If you're going to select a band (and thus apparently create a node) at some time later than compile time, it's only because you allow someone/something who is not the compiler of the code to pick one.
• Never mind how it's communicated to the code, that picking is either in textual from a list, or in a form where the program can reject invalid bands supplied in textual form (such as my serial dialog).
• That means the maker of that software must therefore already know the band names (and thus also how many there are) and have that awareness be in sync with the given version of RadioLib they're talking to. Being able to know how many there are without names doesn't help.

[StevenCellist]

I strongly propose that any maker using LoRaWAN should know how it works, which also includes knowledge about bands. Makers who don't know about bands should not be allowed to develop LoRaWAN products.

[ropg]

In its crudest form:

const char* getBands() {
  return "EU868\tUS915\tCN780\tEU433\tAU915\tCN500\tAS923\tKR920\tIN865";
}

[ropg]

I strongly propose that any maker using LoRaWAN should know how it works, which also includes knowledge about bands. Makers who don't know about bands should not be allowed to develop LoRaWAN products.

Nobody should ever be able to have a dialog box to select a LoRaWAN band? No software should ever exist that doesn't need to be recompiled for each band? Makers of dialog boxes have to manually update their list of bands to match what is supported when they upgrade RadioLib?

[StevenCellist]

You can definitely have a dialog box, but then you should make sure you know a tiny bit about the stack you are working with at the very least. Just one glance over LoRaWANBands.cpp and you know enough. It's even easier than figuring out the details of CSMA or ADR. And yes, you should definitely keep track of updates to the libraries that you include. I just swapped the arguments to beginOTAA() in the last PR. Just updating your dependency is not going to tell you that happened. We will notify in the changelog of breaking changes. And how often do the bands change? Every other day?

[ropg]

As long as everyone is aware that the consequences of the status quo (as well as the proposed change with the enum) are:

• Storing provisioning data means global pointer instead of instance
• Individual providers of provisioning data entry need to keep their own lists

then so be it. Not important enough for further debate.

[jgromes]

@ropg

Storing provisioning data means global pointer instead of instance

The only significant difference I can see between the two is that the former can be placed on stack. Otherwise how exactly does in your use case differ a global instance from a global pointer?

LoRaWANNode node(radioPtr, bandPtr);

As opposed to (in pseudocode):

LoRaWANNode* node = NULL;
(...)
node = new LoRaWANNode(radioPtr, bandPtr);

As a more general point I would prefer the latter in case of runtime-selected band as I can very easily check if it has been instantiated or not. Compare that to having an "incomplete" instance that is not null, but missing its band because it has not been selected yet. Whatever the approach with delaying band selection, you will always have to check that this has not happened. Hard to imagine easier check than if(node).

Individual providers of provisioning data entry need to keep their own lists

That will always be the case. What if the list of in my drop-down list is not "EU868", "US915", etc. but rather something more abstract like "FirstBand", "BackupBand" etc? You will always need mapping from user code to the library API.

Like I wrote above, I can imagine LoRaWANNode::LoRaWANNode(PhysicalLayer* phy, LoRaWANBandNum_t band, uint8_t subBand) being useful. If it's useful to you I will be happy to add it. Otherwise we can close this topic, conversion from some_user_defined_type to LoRaWANBandNum_t is always going to have to be up to the library user ...

[ropg]

I guess I just like things to be consistent, and when things are one way in all the code that's out there. I'd hate my new code to work differently. But pointers are fine, and I've already pointed out the difference in the README to my persist-thing. And added a nice fat

[!WARNING] This library interacts with experimental RadioLib / LoRaWANNode functions that are not in any released versions yet, so for the time being this is only for those using RadioLib fresh off GitHub. This API – along with LoRaWAN functionality in RadioLib more generally – is in beta and subject to change.

[HeadBoffin]

I have a UKHAS colleague who makes devices that automatically switches frequency plans depending on GPS co-ordinates but the USAF has taken to shooting them down of late. He's a PhD at Imperial College (University) so he's down with the code.

I've devices running the same firmware in EU, AU & US that the user can choose, but not with some web or BLE interface because that sort of power requirement isn't a use case for LoRaWAN - it's done via AT commands or a jumper.

I think this comes under the heading of taking an ESP32 centric view coupled with trying to solve problems that rarely exist.