Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

events: Move implementation of events::send() to lib/events #22112

Merged
merged 1 commit into from
Sep 25, 2023

Conversation

pussuw
Copy link
Contributor

@pussuw pussuw commented Sep 21, 2023

Solved Problem

Events have a global, system-wide sequence number, which must be handled atomically, (fetching and incrementing the sequence AND sending the event to uORB must be atomic). Currently in FLAT mode, only one instance of this sequence number exists, so it is OK to have it in px4_platform.

However, in PROTECTED mode px4_platform is instantiated both in kernel- and user spaces, which makes two instances of this sequence number, which causes problems in the mavlink event handling logic.

When mavlink receives and handles events, it expects that:

  • The sequence numbers arrive in order (seq n is followed by n+1 etc)
  • It increments by 1
  • There is only one instance of the sequence number

In PROTECTED mode this is violated, as the kernel and user sequence numbers run freely on their own. This patch fixes the issue by moving the event backend to the kernel and by providing user access to it via ioctl.

Solution

Move events::send() to kernel in PROTECTED build ensuring only a single event sequence number exists in the system

Alternatives

Do the event dispatch logic in some other manner, open to suggestions. This does add a lot of boiler plate code for something extremely simple, i.e. get seq, increment it, set seq to event and send via uorb.

Events have a global, system-wide sequence number, which must be handled
atomically, (fetching and incrementing the sequence AND sending the event
to uORB must be atomic). Currently in FLAT mode, only one instance of this
sequence number exists, so it is OK to have it in px4_platform.

However, in PROTECTED mode px4_platform is instantiated both in kernel-
and user spaces, which makes two instances of this sequence number, which
causes problems in the mavlink event handling logic.

When mavlink receives and handles events, it expects that:
- The sequence numbers arrive in order (seq n is followed by n+1 etc)
- It increments by 1
- There is only one instance of the sequence number

In PROTECTED mode this is violated, as the kernel and user sequence
numbers run freely on their own. This patch fixes the issue by moving
the event backend to the kernel and by providing user access to it via
ioctl.
Copy link
Member

@bkueng bkueng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could partly use the uorb generation, but this is still a bit better.

@bkueng bkueng merged commit 6cb2c17 into PX4:main Sep 25, 2023
86 checks passed
@pussuw pussuw deleted the events_send_to_lib branch September 25, 2023 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants