OS event queue changes

[Christopher Collins <cc...@apache.org>]

Hello all,

Recently a pretty big change was made to Mynewt's event queue model.
The Mynewt documentation gives a good overview of how event queues used
to work
(http://mynewt.apache.org/develop/os/core_os/event_queue/event_queue/).

Here is the old os_event struct:

    struct os_event {
        uint8_t ev_queued;
        uint8_t ev_type;
        void *ev_arg;
        STAILQ_ENTRY(os_event) ev_next;
    };

Here is the new one:

    typedef void os_event_fn(struct os_event *ev);

    struct os_event {
        uint8_t ev_queued;
        os_event_fn *ev_cb;
        void *ev_arg;
        STAILQ_ENTRY(os_event) ev_next;
    };

The difference: the ev_type field was replaced with a callback function
pointer.  A task handler using the old event queue code would dispatch
events based on the event's type.  For example, here is the relevant
part of the old newtmgr task handler:

    while (1) {
        ev = os_eventq_get(&g_nmgr_evq);
        switch (ev->ev_type) {
            case OS_EVENT_T_MQUEUE_DATA:
                nt = (struct nmgr_transport *) ev->ev_arg;
                nmgr_process(nt);
                break;
            case OS_EVENT_T_TIMER:
                ocf = (struct os_callout_func *)ev;
                ocf->cf_func(CF_ARG(ocf));
                break;
        }
    }

Now that the ev_type field has been replaced with a callback function
pointer, the dispatch logic is moved out of the task handler and gets
built into each event.  A task handler now just pulls an event off its
queue and blindly calls its callback function.  A helper function was
added to do just this: os_eventq_run().  As an example, the task handler
for the bleprph application is below:

    static void
    bleprph_task_handler(void *unused)
    {
        while (1) {
            os_eventq_run(&bleprph_evq);
        }
    }

The callback associated with an event is specified when the event gets
initialized.  For example, here are some statically-initialized events
in the nimble host:

    static void ble_hs_event_tx_notify(struct os_event *ev);
    static void ble_hs_event_reset(struct os_event *ev);

    /** OS event - triggers tx of pending notifications and indications. */
    static struct os_event ble_hs_ev_tx_notifications = {
        .ev_cb = ble_hs_event_tx_notify,
    };

    /** OS event - triggers a full reset. */
    static struct os_event ble_hs_ev_reset = {
        .ev_cb = ble_hs_event_reset,
    };

As indicated, the callback function receives a single parameter: a
pointer to the event being processed.  If the event is allocated
dynamically, the callback function probably frees the event.

The above text summarizes what was changed.  Now I want to explain the
rationale.  This change was motivated by a desire to reduce the required
number of tasks in a Mynewt application.  Each extraneous task strains a
device's RAM because it requires the allocation of a dedicated stack.
An application was forced to use a lot of tasks because library packages
would create their own task at initialization time.  For example, nearly
all of the sample apps use the shell and newtmgr packages, both of which
used a dedicated task.

Most of the packages that create a dedicated task don't actually need
one.  These packages don't have any real-time timing requirements, and
therefore no need to preempt the system.  The only reason these packages
were creating a task was to simplify their design and API.  If these
packages could just "piggyback" on some application task, they could
still benefit from a task-oriented design without the cost of a
dedicated task.  However, there is a problem with this idea.  If the
dedicated tasks are eliminated and the work is moved into an application
task, how does the application task know what to do with events meant
for a library?  Even if the application developer is willing to add all
the necessary cases to the task's event dispatch switch, the event types
are ambiguous, since the per-user event type number space is assumed to
be unique for each task.

The solution to this problem is to replace the event type with a
callback function pointer.  Now, the task handler doesn't have to know
whether it just dequeued a shell event or a newtmgr event, it just calls
the associated function.

Packages with timing requirements should still create a dedicated task.
For example, the nimble controller is not affected by this change.
Packages without timing requirements should not create a task.  Instead,
they should initialize their required events at init-time, and provide a
function which allows the application to designate the event queue to be
used.  For example, the nimble host exposes the following function:

    void ble_hs_evq_set(struct os_eventq *evq);

It is a bit of a burden to require the application to set each package's
event queue.  To mitigate this burden, the following function has been
added to the OS:

    void os_eventq_dflt_set(struct os_eventq *evq);

This function designates a default event queue.  If a package is not
explicitly told which event queue to use, it will automatically use the
default.

I hope that mostly makes sense.  Please ask if you have any questions or
if something doesn't seem right.

Thanks,
Chris

Re: OS event queue changes

[Christopher Collins <cc...@apache.org>]

Hi David,

On Mon, Nov 14, 2016 at 01:06:14PM -0500, David G. Simmons wrote:
> Hi Chris,
> 
> Thanks for this detailed write-up on the new os_event and event queue
> handling. 
> 
> I'm working on adding it all to the existing tutorial on creating and
> managing tasks. 
> 
> I'm working on folding this all together with modifying the blinky app
> to use the new event_q and add a shell task to blinky which will, I
> hope, solve the problem of making Blinky a 'playground' again while
> also providing the new event queue model in a tutorial. 
> 
> My main question is if any of the other demo applications have been
> updated to use the new model outlined here or if I'll need to go
> through them all and update them as well. 

The other sample apps have been updated to use the new eventq API.  For
apps which already created their own event queues, the only required
change was to designate the default event queue via a call to
os_eventq_dflt_set().

For apps which did not create an event queue (e.g., slinky, ocf_sample),
an event queue and an additional task were added.  The new task just
calls os_eventq_run() in a loop.

Thanks,
Chris

Re: OS event queue changes

["David G. Simmons" <sa...@mac.com>]

Hi Chris,

Thanks for this detailed write-up on the new os_event and event queue handling. 

I'm working on adding it all to the existing tutorial on creating and managing tasks. 

I'm working on folding this all together with modifying the blinky app to use the new event_q and add a shell task to blinky which will, I hope, solve the problem of making Blinky a 'playground' again while also providing the new event queue model in a tutorial. 

My main question is if any of the other demo applications have been updated to use the new model outlined here or if I'll need to go through them all and update them as well. 

dg

> On Nov 8, 2016, at 8:36 PM, Christopher Collins <cc...@apache.org> wrote:
> 
> Hello all,
> 
> Recently a pretty big change was made to Mynewt's event queue model.
> The Mynewt documentation gives a good overview of how event queues used
> to work
> (http://mynewt.apache.org/develop/os/core_os/event_queue/event_queue/).
> 
> Here is the old os_event struct:
> 
>    struct os_event {
>        uint8_t ev_queued;
>        uint8_t ev_type;
>        void *ev_arg;
>        STAILQ_ENTRY(os_event) ev_next;
>    };
> 
> Here is the new one:
> 
>    typedef void os_event_fn(struct os_event *ev);
> 
>    struct os_event {
>        uint8_t ev_queued;
>        os_event_fn *ev_cb;
>        void *ev_arg;
>        STAILQ_ENTRY(os_event) ev_next;
>    };
> 
> The difference: the ev_type field was replaced with a callback function
> pointer.  A task handler using the old event queue code would dispatch
> events based on the event's type.  For example, here is the relevant
> part of the old newtmgr task handler:
> 
>    while (1) {
>        ev = os_eventq_get(&g_nmgr_evq);
>        switch (ev->ev_type) {
>            case OS_EVENT_T_MQUEUE_DATA:
>                nt = (struct nmgr_transport *) ev->ev_arg;
>                nmgr_process(nt);
>                break;
>            case OS_EVENT_T_TIMER:
>                ocf = (struct os_callout_func *)ev;
>                ocf->cf_func(CF_ARG(ocf));
>                break;
>        }
>    }
> 
> Now that the ev_type field has been replaced with a callback function
> pointer, the dispatch logic is moved out of the task handler and gets
> built into each event.  A task handler now just pulls an event off its
> queue and blindly calls its callback function.  A helper function was
> added to do just this: os_eventq_run().  As an example, the task handler
> for the bleprph application is below:
> 
>    static void
>    bleprph_task_handler(void *unused)
>    {
>        while (1) {
>            os_eventq_run(&bleprph_evq);
>        }
>    }
> 
> The callback associated with an event is specified when the event gets
> initialized.  For example, here are some statically-initialized events
> in the nimble host:
> 
>    static void ble_hs_event_tx_notify(struct os_event *ev);
>    static void ble_hs_event_reset(struct os_event *ev);
> 
>    /** OS event - triggers tx of pending notifications and indications. */
>    static struct os_event ble_hs_ev_tx_notifications = {
>        .ev_cb = ble_hs_event_tx_notify,
>    };
> 
>    /** OS event - triggers a full reset. */
>    static struct os_event ble_hs_ev_reset = {
>        .ev_cb = ble_hs_event_reset,
>    };
> 
> As indicated, the callback function receives a single parameter: a
> pointer to the event being processed.  If the event is allocated
> dynamically, the callback function probably frees the event.
> 
> The above text summarizes what was changed.  Now I want to explain the
> rationale.  This change was motivated by a desire to reduce the required
> number of tasks in a Mynewt application.  Each extraneous task strains a
> device's RAM because it requires the allocation of a dedicated stack.
> An application was forced to use a lot of tasks because library packages
> would create their own task at initialization time.  For example, nearly
> all of the sample apps use the shell and newtmgr packages, both of which
> used a dedicated task.
> 
> Most of the packages that create a dedicated task don't actually need
> one.  These packages don't have any real-time timing requirements, and
> therefore no need to preempt the system.  The only reason these packages
> were creating a task was to simplify their design and API.  If these
> packages could just "piggyback" on some application task, they could
> still benefit from a task-oriented design without the cost of a
> dedicated task.  However, there is a problem with this idea.  If the
> dedicated tasks are eliminated and the work is moved into an application
> task, how does the application task know what to do with events meant
> for a library?  Even if the application developer is willing to add all
> the necessary cases to the task's event dispatch switch, the event types
> are ambiguous, since the per-user event type number space is assumed to
> be unique for each task.
> 
> The solution to this problem is to replace the event type with a
> callback function pointer.  Now, the task handler doesn't have to know
> whether it just dequeued a shell event or a newtmgr event, it just calls
> the associated function.
> 
> Packages with timing requirements should still create a dedicated task.
> For example, the nimble controller is not affected by this change.
> Packages without timing requirements should not create a task.  Instead,
> they should initialize their required events at init-time, and provide a
> function which allows the application to designate the event queue to be
> used.  For example, the nimble host exposes the following function:
> 
>    void ble_hs_evq_set(struct os_eventq *evq);
> 
> It is a bit of a burden to require the application to set each package's
> event queue.  To mitigate this burden, the following function has been
> added to the OS:
> 
>    void os_eventq_dflt_set(struct os_eventq *evq);
> 
> This function designates a default event queue.  If a package is not
> explicitly told which event queue to use, it will automatically use the
> default.
> 
> I hope that mostly makes sense.  Please ask if you have any questions or
> if something doesn't seem right.
> 
> Thanks,
> Chris

--
David G. Simmons
(919) 534-5099
Web <https://davidgs.com/> • Blog <https://davidgs.com/davidgs_blog> • Linkedin <http://linkedin.com/in/davidgsimmons> • Twitter <http://twitter.com/TechEvangelist1> • GitHub <http://github.com/davidgs>
/** Message digitally signed for security and authenticity.  
* If you cannot read the PGP.sig attachment, please go to 
 * http://www.gnupg.com/ <http://www.gnupg.com/> Secure your email!!!
 * Public key available at keyserver.pgp.com <http://keyserver.pgp.com/>
**/
♺ This email uses 100% recycled electrons. Don't blow it by printing!

There are only 2 hard things in computer science: Cache invalidation, naming things, and off-by-one errors.

Re: OS event queue changes

["paul@wrada.com" <pa...@wrada.com>]

Really like this change Chris.  I think its going to make it way easier to
create task-less portable modules.


On 11/8/16, 5:36 PM, "Christopher Collins" <cc...@apache.org> wrote:

>Hello all,
>
>Recently a pretty big change was made to Mynewt's event queue model.
>The Mynewt documentation gives a good overview of how event queues used
>to work
>(http://mynewt.apache.org/develop/os/core_os/event_queue/event_queue/).
>
>Here is the old os_event struct:
>
>    struct os_event {
>        uint8_t ev_queued;
>        uint8_t ev_type;
>        void *ev_arg;
>        STAILQ_ENTRY(os_event) ev_next;
>    };
>
>Here is the new one:
>
>    typedef void os_event_fn(struct os_event *ev);
>
>    struct os_event {
>        uint8_t ev_queued;
>        os_event_fn *ev_cb;
>        void *ev_arg;
>        STAILQ_ENTRY(os_event) ev_next;
>    };
>
>The difference: the ev_type field was replaced with a callback function
>pointer.  A task handler using the old event queue code would dispatch
>events based on the event's type.  For example, here is the relevant
>part of the old newtmgr task handler:
>
>    while (1) {
>        ev = os_eventq_get(&g_nmgr_evq);
>        switch (ev->ev_type) {
>            case OS_EVENT_T_MQUEUE_DATA:
>                nt = (struct nmgr_transport *) ev->ev_arg;
>                nmgr_process(nt);
>                break;
>            case OS_EVENT_T_TIMER:
>                ocf = (struct os_callout_func *)ev;
>                ocf->cf_func(CF_ARG(ocf));
>                break;
>        }
>    }
>
>Now that the ev_type field has been replaced with a callback function
>pointer, the dispatch logic is moved out of the task handler and gets
>built into each event.  A task handler now just pulls an event off its
>queue and blindly calls its callback function.  A helper function was
>added to do just this: os_eventq_run().  As an example, the task handler
>for the bleprph application is below:
>
>    static void
>    bleprph_task_handler(void *unused)
>    {
>        while (1) {
>            os_eventq_run(&bleprph_evq);
>        }
>    }
>
>The callback associated with an event is specified when the event gets
>initialized.  For example, here are some statically-initialized events
>in the nimble host:
>
>    static void ble_hs_event_tx_notify(struct os_event *ev);
>    static void ble_hs_event_reset(struct os_event *ev);
>
>    /** OS event - triggers tx of pending notifications and indications.
>*/
>    static struct os_event ble_hs_ev_tx_notifications = {
>        .ev_cb = ble_hs_event_tx_notify,
>    };
>
>    /** OS event - triggers a full reset. */
>    static struct os_event ble_hs_ev_reset = {
>        .ev_cb = ble_hs_event_reset,
>    };
>
>As indicated, the callback function receives a single parameter: a
>pointer to the event being processed.  If the event is allocated
>dynamically, the callback function probably frees the event.
>
>The above text summarizes what was changed.  Now I want to explain the
>rationale.  This change was motivated by a desire to reduce the required
>number of tasks in a Mynewt application.  Each extraneous task strains a
>device's RAM because it requires the allocation of a dedicated stack.
>An application was forced to use a lot of tasks because library packages
>would create their own task at initialization time.  For example, nearly
>all of the sample apps use the shell and newtmgr packages, both of which
>used a dedicated task.
>
>Most of the packages that create a dedicated task don't actually need
>one.  These packages don't have any real-time timing requirements, and
>therefore no need to preempt the system.  The only reason these packages
>were creating a task was to simplify their design and API.  If these
>packages could just "piggyback" on some application task, they could
>still benefit from a task-oriented design without the cost of a
>dedicated task.  However, there is a problem with this idea.  If the
>dedicated tasks are eliminated and the work is moved into an application
>task, how does the application task know what to do with events meant
>for a library?  Even if the application developer is willing to add all
>the necessary cases to the task's event dispatch switch, the event types
>are ambiguous, since the per-user event type number space is assumed to
>be unique for each task.
>
>The solution to this problem is to replace the event type with a
>callback function pointer.  Now, the task handler doesn't have to know
>whether it just dequeued a shell event or a newtmgr event, it just calls
>the associated function.
>
>Packages with timing requirements should still create a dedicated task.
>For example, the nimble controller is not affected by this change.
>Packages without timing requirements should not create a task.  Instead,
>they should initialize their required events at init-time, and provide a
>function which allows the application to designate the event queue to be
>used.  For example, the nimble host exposes the following function:
>
>    void ble_hs_evq_set(struct os_eventq *evq);
>
>It is a bit of a burden to require the application to set each package's
>event queue.  To mitigate this burden, the following function has been
>added to the OS:
>
>    void os_eventq_dflt_set(struct os_eventq *evq);
>
>This function designates a default event queue.  If a package is not
>explicitly told which event queue to use, it will automatically use the
>default.
>
>I hope that mostly makes sense.  Please ask if you have any questions or
>if something doesn't seem right.
>
>Thanks,
>Chris