Provide standard event handler.

[terrillmoore]

The current upcall-by-name strategy for event handling is really sub-optimal. Each sketch/application must duplicate the case statement and implement the message-based outcall. The Arduino-LoRaWAN library mitigates this, but... it brings in other libraries and is somewhat complex.

However, we don't want to break existing apps.

Although we could have a default event handler in the library with the same name as the user routine, that has caused subtle problems in the past for me, and so I don't think it's robust.

Perhaps this would work.

1. Add new APIs to register event handlers. LMIC_registerEventHandler(pFunction, pClientData) would get all events; LMIC_registerRxHandler(void (*pfn)(uint8_t port, const char *buffer, size_t len, void *pClientData); would register a message receipt handler; and so forth. (Look at Arduino-LoRaWAN onEvent for guidance on what's needed.)
2. The implementation of these API is in a separate .c source file. The source file has a strong reference to a new event handler (or provides the new event handler, with a new name, as part of the source file). This new function would be named LMIC_internalDispatchEvent, and prototype would be in an internal header file (not included by lmic.h).
3. Change the core LMIC code to use a weak reference to onEvent(), and a weak reference to the new LMIC_internalDispatchEvent. If either pointer is non-zero, then the corresponding function is called. If both are non-zero, only the internal function is called (this means that onEvent is no longer a reserved symbol in user code).
4. LMIC_internalDispatchEvent does the work of onEvent() and dispatches as needed.

[frazar]

Each sketch/application must duplicate the case statement and implement the message-based outcall.

I personally don't find the onEvent() function to be cumbersome. It is actually very flexible and efficient.

Registering event handlers would probably be as verbose as the current solution, in terms of lines of code, but with the added cost in terms of RAM and binary size. I expect this kind of user-friendly interface to be offered by Arduino-LoraWAN rather than LMIC.

Of course, this is just my personal opinion.

[terrillmoore]

Unfortunately the name onEvent is just too common; it interferes with use of the LMIC library with other software packages. The LMIC should not have any "magic" or reserved names in the global namespace unless they begin with LMIC_. And more generally, upcalls by name are troublesome and lead to problems in larger software packages. This will be changed as part of the port 224 work.

[frazar]

I agree that the name onEvent() might be clashing with other software packages.

Maybe we could simply rename it LMIC_onEvent()? In this way we avoid the name clashing and the runtime costs.

[terrillmoore]

Maybe we could simply rename it LMIC_onEvent()? In this way we avoid the name clashing and the runtime costs.

Can't do that, breaks existing code. __attribute__((__weak__)) solves this. There is no overhead concern -- this only happens when events happen which is on the order of once per second, much less than the accepted overhead for the SPI accesses. (We don't even cache registers...)

For my uses, I need the callbacks. I'm enclosing that in #if so that it can be disabled for truly tiny applications.

The RAM overhead is also negligible; the LMIC structure can be easily rearranged to save many bytes, because no thought was given to byte/word/dword alignment issues that are typical on ARM. (This is surprising because the original code was for ARM. I'm filing a separate bug for that.)

[frazar]

Oh, I did not understand that this change would be backward compatible!

Also, now that I can see the code, it appears that by defining 'LMIC_CFG_disable_user_events' one can opt-out from the cost of storing callback pointers.