app event manager section placement sort issue

[Zordon]

2100.app_event_manager_issue.zip

Hello,

I hope you are having a nice day. I recently updated from ncs v1.6.1 to v2.2.0. My application is based on the "Application Event Manager" module. I have found that section placement of the event subscribers is being sorted alphabetically by the linker. This has the side effect of potentially having subscriber sections interspersed if there happens to be overlap in the event names. For example, say an application has two events ("event_rx", and "event_rx_error"). The zephyr.map will be generated with sections in the following order:

event_subscribers_event_rx_a
event_subscribers_event_rx_error_a   --------------------> WHOOPS!
event_subscribers_event_rx_error_p10
event_subscribers_event_rx_error_z
event_subscribers_event_rx_p10
event_subscribers_event_rx_z

As noted, these sections are listed alphabetically, but the Application Event Manager logic does not handle interspersed subscriber sections.

I have attached an example using the application event manager reference example. In the example, I changed the "control_event" to "measurement_event_control_event". I added a text file containing how the zephyr.map file ends up not laid out properly. In the reference material, nordic (by convention) ends their events with "_event". Following this practice can mitigate the section placement issue to a degree, but if the name of one event overlaps with the name of another event, this section placement issue can still occur. 

Is there a way to explicitly eliminate this section placement issue? 

(aside from manually enforcing naming convention rules)

Additionally, I noticed on ncs v2.2.0 while debugging event listeners (an activity which blocks the system work queue) zephyr timer callbacks (in my case running every 10ms) that create events will hit event allocation failure. Is this mitigatable, perhaps by instead using delayed work items instead of timer callbacks? What is best practice here?

Thank you very much for your time!

[Radoslaw Koppel]

Hello. I am working on it. As a temporary solution I would propose to keep our naming pattern where the event always has _event postfix. Then it would work:

event_subscribers_rx_error_event_a   --------------------> no more problem
event_subscribers_rx_error_event_p10
event_subscribers_rx_error_event_z
event_subscribers_rx_event_a
event_subscribers_rx_event_event_p10
event_subscribers_rx_event_z

I have an idea how to allow any naming pattern in the event manager but it would take some time. Sorry for inconvenience. I have to admit we have not tested this solution with different naming pattern.

[Zordon]

Hello,

Thank you very much for your response. The "_event" postfix definately reduces the risk of encountering the issue, but unfortunately if one event happens to contain the name of a second event the problem will still appear ("control_event" and "measurement_event_control_event" for example). Perhaps a safer method would be to strip underscores from the event names. Or perhaps a quick fix would be to add __file__ and __line__ to the event names to prevent overlap in the event names..

Also, I was wondering what you would recommend as best practice:

Zordon said:

Additionally, I noticed on ncs v2.2.0 while debugging event listeners (an activity which blocks the system work queue) zephyr timer callbacks (in my case running every 10ms) that create events will hit event allocation failure. Is this mitigatable, perhaps by instead using delayed work items instead of timer callbacks? What is best practice here?

Thanks again- I really appreciate the support.

[Radoslaw Koppel]

The sample with measurement_event_control_event and control event is not the best:

control_event_a
control_event_p10
control_event_z
measurement_event_control_event_a
measurement_event_control_event_p10
measurement_event_control_event_z

But I get what you mean:

control_event_a
control_event_measurement_event_a
control_event_measurement_event_p10
control_event_measurement_event_z
control_event_p10
control_event_z

Anyway - it is just a workaround and was not designed to be bulletproof. Creating events like <name>_event_<other_name>_<event> seems not to serve for code readability. Just use control_event and control_measurement_event.

The preferred solution is in a PR: https://github.com/nrfconnect/sdk-nrf/pull/10093 - and I expect this one to allow any naming scheme to be used.

I expect it to be present on the next release. It should be possible to just cherry-pick it on the top of the last release.

Thank you for reporting this issue. It would be probably much longer unseen without that.

When it comes to debugging - the best solution seems to be just to stop the timers when the core is stopped. To do that you have to use gdb-hooks (hook-stop and hook-continue) and call the functions that would stop and restart the timer from them.

Another possibility is to reimplement event_manager allocation functions not to fail on allocation failure but just to do some busy_waiting and then retry. I don't think this should go into release code, but should be a workaround when debugging - but then expect that when gdb would call continue all the events that are there in the event queue would be processed one after another.

[Zordon]

Much appreciated. Thanks again.