Remove duplicate code and extra memory allocations

[TD-er]

I also did split a lot of raw command calls into separate functions to make them better readable and also reduce the number of strings used in the code.

Almost all functions with String type arguments did not use const String reference. This means every call does perform a deepcopy (and thus lots of memory re-allocations)

The library as it is in this PR is also being used now in ESPeasy. See https://github.com/letscontrolit/ESPEasy/pull/2539

Later on, I will change it into an async version so the controller does not have to wait for the packet to be sent or wait for a long time for an acknowledgement if asked for. But such a change will be a breaking change in how to interface with the library. When that's ready, I will again make a pull request.

[jpmeijers]

Ahh excellent. Thanks a lot for this impressive improvement!

Regarding the blocking vs. async version, perhaps the async one should be a fork. For beginners in the embedded (Arduino) world blocking functions are much easier to understand and use. The goal of this library would rather be to keep things simple rather and super efficient. But async would also make sense for a next step in learning.

(Such a pity github doesn't pick up the diff correctly. Must be due to indentation changes.)

[TD-er]

You can change the view to ignore white space changes.

[jpmeijers]

Thanks again. You're teaching me (us) a lot here. Not only Github features but also your good coding style.

[TD-er]

Thanks.

Just wanted to let you know I also changed this behavior as I think it may have been a bug. Please have a look at it:

The channels were said to be disabled in the comment, but they weren't.

[jpmeijers]

Yes I believe you are right that there was a bug. Default EU frequency plan should only have the first three channels enabled on their default frequencies.

As a side not I see you changed to for-loops to configure the channels. The TTN library also does this. Personally I prefer not to wrap this in loops as writing them out line by line is much more readable.

Example:

setChannelEnabled(0, true);
setChannelEnabled(1, true);
setChannelEnabled(2, true);
setChannelEnabled(3, false);
setChannelEnabled(4, false);
setChannelEnabled(5, false);
setChannelEnabled(6, false);
setChannelEnabled(7, false);

versus

for(int channel=0; channel<8; channel++) {
  if(channel<3) {
    setChannelEnabled(channel, true);
  } else {
    setChannelEnabled(channel, false);
  }
}

But then again the for-loops likely use less program space, and because normal users won't look at the library source the space gained is worth the readability trade-off. Is this however true? Doesn't the overhead of the for-loop and if-checks require more space than the unwrapped loop - with this number of loops?

https://en.wikipedia.org/wiki/Loop_unrolling

[TD-er]

I don't know what threshold the compiler uses to use loop unrolling. I can imagine it also is highly depending on the platform whether it is a speed optimization or not. The ESP doesn't have a long pipeline, nor branch prediction, so the gain of loop unrolling may be close to 0. By the way your "versus" example has a bug in it ;) (starting at 3, so first if will never be true) Just checked to be sure, but it is not a cut'n-paste of my changes ;)

[jpmeijers]

Haha no, those code fragments were a quick rewrite by me taking your screenshot as example.