[WIP] Multiple POCSAG messages at once (fixes #743)

[h3ndrik]

I'd like to request some comments on how to restructure the code to implement #743 .

Is another class "PagerMessage" okay? May I also move some code there?
I assume we lose sync if we just loop through the messages and calculate while we're sending them out? That would make it necessary to calculate beforehand and have a large buffer.
Do we hide the buffer handling inside the library?
How much attention do we pay to array sizes and not to overstep them?
Also: i ripped apart some of the code and put it into smaller/different functions.
What do i do with the message data strings? Do we want references to them inside the PagerMessage? Or copy them? I'm not sure how people use this. The strings might get out of scope if done poorly.
Is there any sane way to arrange messages so we use the leftover frames after the data of the last message ended and before the address frame that starts the next message? I guess that would be quite some work. I'd rather just fill them with IDLE words.

This code is not ready yet. I'm sorry, my head hurts from all the details, I need a break and I might have made several one-off errors. It seems to be working, though.
I just wanted someone to look at it and tell me if this is the right direction, before cleaning all this up and making it work properly.

[jgromes]

Thanks for this contribution! I actually started sketching out my own, but it's not as advanced as yours, so let's stick to it. For context, my approach was to add two new methods, addPacket and sendBatch. addPacket adds a new packet to an internal buffer, does all the calculations, encoding etc. Then the user can send the message by calling sendBatch. The current interface would remain, transmit would just call addPacket and sendBatch.

To answer your questions:

Is another class "PagerMessage" okay? May I also move some code there?

Yes, defnitely (there is a precedent in the AX.25 frame class). My approach did not use a special class, but I was already considering it and would likely ended up adding it.

I assume we lose sync if we just loop through the messages and calculate while we're sending them out? That would make it necessary to calculate beforehand and have a large buffer.

I'm not sure how that would work, you would first need to order the messages by destination because part of the destination address also determines position in the buffer. Overall this would lead to a new interrupt-driven implementation of Pager::write() as opposed to the current blocking one.

Do we hide the buffer handling inside the library?

For now, that is the approach. It's not ideal, and in the future I would like to add an option for the user to supply their own "working buffer" for the library, but now that's the baseline.

How much attention do we pay to array sizes and not to overstep them?

As much as is needed to gurantee overflows or memory leaks do not happen.

i ripped apart some of the code and put it into smaller/different functions.

That's a beter approach than the previous "single blob" :)

What do i do with the message data strings?

I would like to keep the current simple API. It's not necessary to copy the string contents, the library has to trust the user is providing valid data (apart from some basic NULL checks).

Is there any sane way to arrange messages so we use the leftover frames after the data of the last message ended and before the address frame that starts the next message?

Probably not worth the effort (I tried to do it in my approach and it can be done, but I don't have it working yet). Currently, if you want to send out multiple messages, you will always send at least a preamble and one batch. The next improvement is to send one preamble and then append batches in the order that user added messages. This already saves one preamble per each message, which is a significant improvement. I guess it also depends on whether you expect to send messages to destination addresses that are close to each other in the address space (because three address bits are position within batch).

[h3ndrik]

my approach was to add two new methods, addPacket and sendBatch

I like that idea. I'd try not to call it sendBatch though, because the term batch is used in the pocsag protocol.
I'm a bit lost with the internal buffer right now. It would need to be either dynamic or we need to tell the user what to expect. I think i read you're allowed to send for 30 seconds consecutively (including preamble and everything), worst case is 2400 baud(?) and a microcontroller with little memory. I'm not sure what users of this library expect. I can try and make the buffer grow with each addPacket, but that would probably mean just re-implementing arduino Strings.

you would first need to order the messages by destination because part of the destination address also determines position in the buffer.

I think this is a misunderstanding. It just determines in which of the 8(?) words in every batch the pager listens (for the remaining part of its address). But it does so in every batch/message / it starts over every 16 words. Order of the messages doesn't matter. We just fill the remaining words of a batch with IDLE and start over with any new message.

a new interrupt-driven implementation

Okay, i don't really want to implement this. It works fine as is.

Probably not worth the effort [...] destination addresses that are close to each other in the address space (because three address bits are position within batch).

I think the whole way pocsag works, is because in ancient times there were lots of tone alerts that were just 1 word(?) long. Or other short messages. You could just bridge the gaps between longer messages with those. I think people nowadays use this to send longer ascii messages and for those it's really not worth the effort. Ordering those would be a proper optimization problem.

Thanks for answering all those questions.

[jgromes]

I like that idea. I'd try not to call it sendBatch though, because the term batch is used in the pocsag protocol.

Fine by me if you find a better name ;) Guess it would have to be sendBatches to be correct, but that's a bit weird from the user side.

It would need to be either dynamic or we need to tell the user what to expect

Dynamic memory is accepted, assuming you also provide means to disable, even if just by having a large static buffer. See e.g.:

you're allowed to send for 30 seconds consecutively (including preamble and everything), worst case is 2400 baud(?)

Those seem like reasonable sizing assumptions. In reality RadioLib does not enforce either of these limits, so if someone decides to have a 19k2 bps custom POCSAG network running only RadioLib devices, there's not much we can do to stop them.

and a microcontroller with little memory. I'm not sure what users of this library expect.

The lowest-spec platform officially supported by RadioLib is Arduino Uno/ATmega328P with 32kB program memory. The unfortunate reality is that as RadioLib evolves, it slowly increases in size and eventually will outgrow this 20 year old device. The good news is that it was mostly replaced by ESP8266/ESP32 which has more than enough memory, so don't worry about dynamic allocations or large buffers.

I can try and make the buffer grow with each addPacket

Please don't, dynamically allocating memory is not great, but safe if handled correctly. Changing the allocated memory size with each call just invites memory fragmentation.

I think this is a misunderstanding. It just determines in which of the 8(?) words in every batch the pager listens (for the remaining part of its address). But it does so in every batch/message / it starts over every 16 words

Sooo .... is the message itself also "spread out" throughout the batches, or does it only start at the address-determined position and the continue onwards? This is actually not clear in the specification. RadioLib implements the second approach, but that also means that positions after a message starts can no longer be used for other messages. For example, if I have a message that starts at code word position 12, and is 6 words long, then it will occupy code words 12 - 15 in the first batch and 0 - 1 in the following batch. But that also means that in the second batch we cannot fit a message that would start at code word 0.

[jgromes]

This PR has been left unchanged for quite a few months now, and it also has conflicts with the upstream, so I will close it for now. If at some point this continues being worked on, feel free to reopen.