Expose implementation internals of classes, so that it's easier to customize them

[jasiek]

Hey,

I'm working on an open-source POCSAG pager (https://github.com/jasiek/ospager-firmware). This requires a little bit of customization of existing libraries, which I think I can accomplish through subclassing. Unfortunately there are parts (https://github.com/jgromes/RadioLib/blob/master/src/protocols/Pager/Pager.cpp#L13) whose visibility is restricted to this compilation unit.

If I submit a PR to remove the static qualifier, will that be accepted? Perhaps we could wrap that in RADIOLIB_GODMODE ifdef?
Alternatively, is there a better way to accomplish what I want to do?

[jasiek]

I've solved for this by using extern, but there still is a problem accessing PhysicalLayer.getMod() from derived classes. Should I submit a PR to address that?

[jgromes]

The readBitInstance pointer is made static for a reason - we need to rely on this pointer to be pointing correctly to a valid instance of PhysicalLayer, otherwise things will go very wrong - also as the comment states, it's a hacky way to go about reading the bits. I don't think it should be exposed, hence the static keyword. PhysicalLayer::getMod() falls under similar logic: it lets you poke around the library internals.

So I guess the better question is: what exactly are you trying to achieve? Maybe there's a way to make the API more flexible so that it suits your use-case?

[jasiek]

I've subclassed PagerClient to be able to receive messages for multiple POCSAG addresses (https://github.com/jasiek/ospager-firmware/blob/main/src/MultiAddressPagerClient.cpp). This way, in addition to regular messages, I can receive time synchronisations and other broadcasts.

PagerClient relies on friend access to get to getMod(). Now that I looked into it again, I think there's a workaround, which I've just checked in, it's a little hacky, but it should work.

[jasiek]

Maybe the way to do it would be to supply an address-matching function to PagerClient - this would let the use receive messages for any set of addresses using whatever mechanism they want to use.

[jgromes]

I think this would be better addressed by extending RadioLib's startReceive method. Currently it only takes a single address/mask, which I hoped would be enough for all use cases - though I guess there can be address combinations that cannot be matched with a single mask. However, we can easily add another method startReceive(uint32_t pin, uint32_t* addr, uint32_t* mask, size_t numAddr) to set numAddr address/mask combinations. If you would be willing to send a pull request addressing this, I will happilly merge it ;)

[jasiek]

Cool. I'll do that.

…

On Sat, Mar 2, 2024, 09:19 Jan Gromeš ***@***.***> wrote: I think this would be better addressed by extending RadioLib's startReceive method. Currently it only takes a single address/mask, which I hoped would be enough for all use cases - though I guess there can be address combinations that cannot be matched with a single mask. However, we can easily add another method startReceive(uint32_t pin, uint32_t* addr, uint32_t* mask, size_t numAddr) to set numAddr address/mask combinations. If you would be willing to send a pull request addressing this, I will happilly merge it ;) — Reply to this email directly, view it on GitHub <#994 (reply in thread)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAJFLVKC7T7UNI2AGAGO33YWGDQ5AVCNFSM6AAAAABEAGFHDSVHI2DSMVQWIX3LMV43SRDJONRXK43TNFXW4Q3PNVWWK3TUHM4DMNJQGM2TK> . You are receiving this because you modified the open/close state.Message ID: ***@***.***>