Fix userallocatedevent imp #12319

maciejbocianski · 2020-01-28T09:21:16Z

Summary of changes

Fix user allocated events implementation:

store delay and period inside UserAllocatedEvent and set it at every post
fix user allocated events canceling
update tests accordingly

Impact of changes

Migration actions required

Documentation

Pull request type

[X] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[X] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers

@jamesbeyond @kjbracey-arm

ciarmcom · 2020-01-28T10:00:43Z

@maciejbocianski, thank you for your changes.
@jamesbeyond @ARMmbed/mbed-os-core @ARMmbed/mbed-os-test @ARMmbed/mbed-os-maintainers please review.

bulislaw · 2020-02-03T15:44:45Z

events/source/equeue.c

@@ -402,6 +404,10 @@ void equeue_post_user_allocated(equeue_t *q, void (*cb)(void *), void *p)
    unsigned tick = equeue_tick();
    e->cb = cb;
    e->target = tick + e->target;
+    // for user allocated events use event id to track event state


That is not very readable, can we use some kind of enum maybe? So we don't introduce magic numbers. Also are you sure that hijacking of this fields is ok (to be fair i'm not 100% sure how it's being used)

True, I will use enum.

Hijacking id field should be OK - all user_allocated equeue APIs (and whole equeue implementation) use event address instead event id when dealing with user allocated events.

@bulislaw I have reviewed equeue code once more and found one place where id was altered without checking event type. Already fixed see 7215f99

bulislaw · 2020-02-03T16:17:19Z

events/UserAllocatedEvent.h

@@ -260,17 +262,22 @@ class UserAllocatedEvent<F, void(ArgTs...)> {
            return false;
        }
        core_util_atomic_incr_u8(&_post_ref, 1);
+        equeue_event_delay(&_e + 1, _delay);


I think that changes the behavior? As before we could change delay and period of already posted events?

It wasn't allowed previously. It was guarded by MBED_ASSERT(!_post_ref).
It isn't a valid use case to change delay/period while dispatching is ongoing.

0xc0170 · 2020-02-05T15:19:44Z

PR was updated, please review

0xc0170 · 2020-02-07T15:20:47Z

CI started

mbed-ci · 2020-02-07T17:02:58Z

Test run: FAILED

Summary: 2 of 4 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-GCC_ARM
jenkins-ci/mbed-os-ci_build-IAR

maciejbocianski · 2020-02-10T07:14:39Z

not related - problems with IAR licence,

Compile [ 20.0%]: ip_fsc.c
Compile [ 20.1%]: common_functions.c
[ERROR] Fatal error[LMS001]: License check failed. Use the IAR License Manager to

Compile [ 12.5%]: QUECTEL_BG96_CellularStack.cpp
[ERROR] 
Failed to build library

0xc0170 · 2020-02-10T10:42:27Z

Ci restarted

mbed-ci · 2020-02-10T11:17:57Z

Test run: FAILED

Summary: 1 of 4 test jobs failed
Build number : 2
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-GCC_ARM

0xc0170 · 2020-02-10T12:19:03Z

Java error, restarted

mbed-ci · 2020-02-10T15:06:17Z

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 3
Build artifacts

0xc0170 · 2020-02-10T15:55:00Z

There was one more CI job created 👀

mbed-ci · 2020-02-10T16:59:29Z

Test run: FAILED

Summary: 1 of 11 test jobs failed
Build number : 4
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_greentea-test

0xc0170 · 2020-02-13T12:06:23Z

Ci completed, reviews required

adbridge · 2020-03-06T14:01:36Z

This ran through ci 22 days ago so needs to be re-run

mbed-ci · 2020-03-06T16:44:36Z

Test run: FAILED

Summary: 1 of 7 test jobs failed
Build number : 6
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_cloud-client-pytest

0xc0170 · 2020-03-09T07:16:47Z

Client restarted

jamesbeyond requested a review from kjbracey January 28, 2020 09:47

ciarmcom requested review from jamesbeyond and a team January 28, 2020 10:00

ciarmcom added the needs: review label Jan 28, 2020

bulislaw reviewed Feb 3, 2020

View reviewed changes

maciejbocianski force-pushed the fix_userallocatedevent_imp branch from 37b2ad1 to 1f9b5de Compare February 4, 2020 20:51

maciejbocianski requested a review from bulislaw February 5, 2020 15:36

bulislaw previously approved these changes Feb 7, 2020

View reviewed changes

mergify bot added needs: CI and removed needs: review labels Feb 7, 2020

mergify bot added needs: work and removed needs: CI labels Feb 10, 2020

0xc0170 added needs: CI and removed needs: work labels Feb 10, 2020

0xc0170 removed the request for review from a team February 10, 2020 15:56

mergify bot added needs: work and removed needs: CI labels Feb 10, 2020

0xc0170 added release-version: 6.0.0-alpha-3 and removed release-version: 6.0.0-alpha-2 Second pre-release version of 6.0.0 labels Feb 27, 2020

0xc0170 approved these changes Feb 28, 2020

View reviewed changes

jamesbeyond approved these changes Feb 28, 2020

View reviewed changes

bulislaw approved these changes Mar 3, 2020

View reviewed changes

adbridge added needs: CI and removed needs: review labels Mar 6, 2020

mergify bot added ready for merge needs: CI needs: work and removed needs: CI labels Mar 6, 2020

0xc0170 removed the ready for merge label Mar 9, 2020

0xc0170 added needs: CI and removed needs: work labels Mar 9, 2020

mergify bot added ready for merge needs: work and removed needs: CI labels Mar 9, 2020

0xc0170 added mergify skip needs: CI and removed needs: work ready for merge labels Mar 9, 2020

adbridge merged commit d5f034b into ARMmbed:master Mar 11, 2020

adbridge removed needs: CI mergify skip labels Mar 11, 2020

maciejbocianski mentioned this pull request Mar 20, 2020

period of event1 is reset by cancel ARMmbed/mbed-os-5-docs#1190

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix userallocatedevent imp #12319

Fix userallocatedevent imp #12319

maciejbocianski commented Jan 28, 2020

ciarmcom commented Jan 28, 2020

bulislaw Feb 3, 2020

maciejbocianski Feb 4, 2020 •

edited

Loading

maciejbocianski Feb 4, 2020

bulislaw Feb 3, 2020

maciejbocianski Feb 4, 2020

0xc0170 commented Feb 5, 2020

0xc0170 commented Feb 7, 2020

mbed-ci commented Feb 7, 2020

maciejbocianski commented Feb 10, 2020

0xc0170 commented Feb 10, 2020

mbed-ci commented Feb 10, 2020

0xc0170 commented Feb 10, 2020

mbed-ci commented Feb 10, 2020

0xc0170 commented Feb 10, 2020

mbed-ci commented Feb 10, 2020

0xc0170 commented Feb 13, 2020

adbridge commented Mar 6, 2020

mbed-ci commented Mar 6, 2020

0xc0170 commented Mar 9, 2020

Fix userallocatedevent imp #12319

Fix userallocatedevent imp #12319

Conversation

maciejbocianski commented Jan 28, 2020

Summary of changes

Impact of changes

Migration actions required

Documentation

Pull request type

Test results

Reviewers

ciarmcom commented Jan 28, 2020

Choose a reason for hiding this comment

maciejbocianski Feb 4, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xc0170 commented Feb 5, 2020

0xc0170 commented Feb 7, 2020

mbed-ci commented Feb 7, 2020

maciejbocianski commented Feb 10, 2020

0xc0170 commented Feb 10, 2020

mbed-ci commented Feb 10, 2020

0xc0170 commented Feb 10, 2020

mbed-ci commented Feb 10, 2020

0xc0170 commented Feb 10, 2020

mbed-ci commented Feb 10, 2020

0xc0170 commented Feb 13, 2020

adbridge commented Mar 6, 2020

mbed-ci commented Mar 6, 2020

0xc0170 commented Mar 9, 2020

maciejbocianski Feb 4, 2020 •

edited

Loading