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.

Sign up for GitHub

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

Fix userallocatedevent imp #12319

Merged
merged 5 commits into from
Mar 11, 2020
+116 −53
Merged

Fix userallocatedevent imp #12319

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
acfdc58
equeue: skip equeue_incid call for user allocated events while dispat…
maciejbocianski Feb 4, 2020
b4fac71
user allocated events: fix event cancelling
maciejbocianski Jan 24, 2020
bc89974
UserAllocatedEvent: set delay/period at event posting
maciejbocianski Jan 24, 2020
784393a
equeue tests update
maciejbocianski Jan 24, 2020
83a0783
queue test update
maciejbocianski Jan 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 16 additions & 12 deletions TESTS/events/equeue/main.cpp
View file
Original file line number Diff line number Diff line change
@@ -1003,23 +1003,24 @@ static void test_equeue_user_allocated_event_post()

uint8_t touched = 0;
user_allocated_event e1 = { { 0, 0, 0, NULL, NULL, NULL, 0, -1, NULL, NULL }, 0 };
user_allocated_event e2 = { { 0, 0, 0, NULL, NULL, NULL, 1, -1, NULL, NULL }, 0 };
user_allocated_event e3 = { { 0, 0, 0, NULL, NULL, NULL, 1, -1, NULL, NULL }, 0 };
user_allocated_event e4 = { { 0, 0, 0, NULL, NULL, NULL, 1, -1, NULL, NULL }, 0 };
user_allocated_event e2 = { { 0, 0, 0, NULL, NULL, NULL, 10, 10, NULL, NULL }, 0 };
user_allocated_event e3 = { { 0, 0, 0, NULL, NULL, NULL, 10, 10, NULL, NULL }, 0 };
user_allocated_event e4 = { { 0, 0, 0, NULL, NULL, NULL, 10, 10, NULL, NULL }, 0 };
user_allocated_event e5 = { { 0, 0, 0, NULL, NULL, NULL, 0, -1, NULL, NULL }, 0 };

TEST_ASSERT_NOT_EQUAL(0, equeue_call(&q, simple_func, &touched));
TEST_ASSERT_EQUAL_INT(0, equeue_call(&q, simple_func, &touched));
TEST_ASSERT_EQUAL_INT(0, equeue_call(&q, simple_func, &touched));
TEST_ASSERT_NOT_EQUAL(0, equeue_call_every(&q, 10, simple_func, &touched));
TEST_ASSERT_EQUAL_INT(0, equeue_call_every(&q, 10, simple_func, &touched));
TEST_ASSERT_EQUAL_INT(0, equeue_call_every(&q, 10, simple_func, &touched));

equeue_post_user_allocated(&q, simple_func, &e1.e);
equeue_post_user_allocated(&q, simple_func, &e2.e);
equeue_post_user_allocated(&q, simple_func, &e3.e);
equeue_post_user_allocated(&q, simple_func, &e4.e);
equeue_post_user_allocated(&q, simple_func, &e5.e);
equeue_cancel_user_allocated(&q, &e3.e);
equeue_cancel_user_allocated(&q, &e3.e);

equeue_dispatch(&q, 1);
equeue_dispatch(&q, 11);

TEST_ASSERT_EQUAL_UINT8(1, touched);
TEST_ASSERT_EQUAL_UINT8(1, e1.touched);
@@ -1028,13 +1029,16 @@ static void test_equeue_user_allocated_event_post()
TEST_ASSERT_EQUAL_UINT8(1, e4.touched);
TEST_ASSERT_EQUAL_UINT8(1, e5.touched);

equeue_dispatch(&q, 10);
e3.e.target = 10; // set target as it's modified by equeue_call
e3.e.period = 10; // set period as it's reset by equeue_cancel
equeue_post_user_allocated(&q, simple_func, &e3.e);
equeue_dispatch(&q, 101);

TEST_ASSERT_EQUAL_UINT8(1, touched);
TEST_ASSERT_EQUAL_UINT8(11, touched);
TEST_ASSERT_EQUAL_UINT8(1, e1.touched);
TEST_ASSERT_EQUAL_UINT8(1, e2.touched);
TEST_ASSERT_EQUAL_UINT8(0, e3.touched);
TEST_ASSERT_EQUAL_UINT8(1, e4.touched);
TEST_ASSERT_EQUAL_UINT8(11, e2.touched);
TEST_ASSERT_EQUAL_UINT8(10, e3.touched);
TEST_ASSERT_EQUAL_UINT8(11, e4.touched);
TEST_ASSERT_EQUAL_UINT8(1, e5.touched);

equeue_destroy(&q);
73 changes: 55 additions & 18 deletions TESTS/events/queue/main.cpp
View file
Original file line number Diff line number Diff line change
@@ -362,14 +362,18 @@ void mixed_dynamic_static_events_queue_test()

EventTest e1_test;
Event<void()> e1 = queue.event(&e1_test, &EventTest::f0);
e1.delay(10);
e1.period(10);
int id1 = e1.post();
TEST_ASSERT_NOT_EQUAL(0, id1);
EventTest e2_test;
Event<void()> e2 = queue.event(&e2_test, &EventTest::f1, 3);
e2.period(10);
int id2 = e2.post();
TEST_ASSERT_NOT_EQUAL(0, id2);
EventTest e3_test;
Event<void()> e3 = queue.event(&e3_test, &EventTest::f5, 1, 2, 3, 4, 5);
e3.period(10);
int id3 = e3.post();
TEST_ASSERT_NOT_EQUAL(0, id3);

@@ -390,8 +394,11 @@ void mixed_dynamic_static_events_queue_test()
TEST_ASSERT_EQUAL(false, ue0.try_call());
ue1.call_on(&queue);
TEST_ASSERT_EQUAL(false, ue1.try_call());
ue2.period(10);
ue2.call_on(&queue);
TEST_ASSERT_EQUAL(false, ue2.try_call());
ue3.period(10);
ue3.delay(50);
ue3.call_on(&queue);
TEST_ASSERT_EQUAL(false, ue3.try_call());
ue4.call_on(&queue);
@@ -400,21 +407,36 @@ void mixed_dynamic_static_events_queue_test()
ue4.cancel();
e2.cancel();

queue.dispatch(1);
queue.dispatch(101);

TEST_ASSERT_EQUAL(true, touched);
TEST_ASSERT_EQUAL(1, ue1_test.counter);
TEST_ASSERT_EQUAL(3, ue2_test.counter);
TEST_ASSERT_EQUAL(15, ue3_test.counter);
TEST_ASSERT_EQUAL(33, ue2_test.counter);
TEST_ASSERT_EQUAL(90, ue3_test.counter);
TEST_ASSERT_EQUAL(0, ue4_test.counter);
TEST_ASSERT_EQUAL(1, e1_test.counter);
TEST_ASSERT_EQUAL(10, e1_test.counter);
TEST_ASSERT_EQUAL(0, e2_test.counter);
TEST_ASSERT_EQUAL(15, e3_test.counter);
TEST_ASSERT_EQUAL(165, e3_test.counter);

// user allocated event have to be canceled(removed from the queue) before destruction
// cancel all periodic user events
ue2.cancel();
ue3.cancel();
}
}


static EventQueue g_queue(0);
static auto ue0 = g_queue.make_user_allocated_event(func0);
static EventTest test1;
static auto ue1 = make_user_allocated_event(&test1, &EventTest::f0);
static EventTest test2;
static auto ue2 = g_queue.make_user_allocated_event(&test2, &EventTest::f1, 3);
static EventTest test3;
static auto ue3 = make_user_allocated_event(&test3, &EventTest::f5, 1, 2, 3, 4, 5);
static EventTest test4;
static auto ue4 = g_queue.make_user_allocated_event(&test4, &EventTest::f5, 1, 2, 3, 4, 5);


/** Test that static queue executes user allocated events.
*
@@ -428,15 +450,20 @@ void static_events_queue_test()
Event<void()> e0 = g_queue.event(func0);
TEST_ASSERT_EQUAL(0, e0.post());

auto ue0 = g_queue.make_user_allocated_event(func0);
EventTest test1;
auto ue1 = make_user_allocated_event(&test1, &EventTest::f0);
EventTest test2;
auto ue2 = g_queue.make_user_allocated_event(&test2, &EventTest::f1, 3);
EventTest test3;
auto ue3 = make_user_allocated_event(&test3, &EventTest::f5, 1, 2, 3, 4, 5);
EventTest test4;
auto ue4 = g_queue.make_user_allocated_event(&test4, &EventTest::f5, 1, 2, 3, 4, 5);
ue0.delay(100);
ue0.period(200);

ue1.delay(100);
ue1.period(200);

ue2.delay(100);
ue2.period(200);

ue3.delay(100);
ue3.period(200);

ue4.delay(100);
ue4.period(200);

ue0.call();
TEST_ASSERT_EQUAL(false, ue0.try_call());
@@ -448,16 +475,26 @@ void static_events_queue_test()
TEST_ASSERT_EQUAL(false, ue3.try_call());
ue4.call();
ue4.cancel();
ue4.cancel();
TEST_ASSERT_EQUAL(true, ue4.try_call());
g_queue.cancel(&ue4);
g_queue.cancel(&ue4);

g_queue.dispatch(1);
g_queue.dispatch(400);

TEST_ASSERT_EQUAL(1, test1.counter);
TEST_ASSERT_EQUAL(3, test2.counter);
TEST_ASSERT_EQUAL(15, test3.counter);
TEST_ASSERT_EQUAL(2, test1.counter);
TEST_ASSERT_EQUAL(6, test2.counter);
TEST_ASSERT_EQUAL(30, test3.counter);
TEST_ASSERT_EQUAL(0, test4.counter);

ue4.delay(1);
TEST_ASSERT_EQUAL(true, ue4.try_call());
g_queue.dispatch(1);

TEST_ASSERT_EQUAL(2, test1.counter);
TEST_ASSERT_EQUAL(6, test2.counter);
TEST_ASSERT_EQUAL(30, test3.counter);
TEST_ASSERT_EQUAL(15, test4.counter);
}

// Test setup
34 changes: 19 additions & 15 deletions UNITTESTS/events/equeue/test_equeue.cpp
View file
Original file line number Diff line number Diff line change
@@ -1072,23 +1072,24 @@ TEST_F(TestEqueue, test_equeue_user_allocated_event_post)

uint8_t touched = 0;
user_allocated_event e1 = { { 0, 0, 0, NULL, NULL, NULL, 0, -1, NULL, NULL }, 0 };
user_allocated_event e2 = { { 0, 0, 0, NULL, NULL, NULL, 1, -1, NULL, NULL }, 0 };
user_allocated_event e3 = { { 0, 0, 0, NULL, NULL, NULL, 1, -1, NULL, NULL }, 0 };
user_allocated_event e4 = { { 0, 0, 0, NULL, NULL, NULL, 1, -1, NULL, NULL }, 0 };
user_allocated_event e2 = { { 0, 0, 0, NULL, NULL, NULL, 10, 10, NULL, NULL }, 0 };
user_allocated_event e3 = { { 0, 0, 0, NULL, NULL, NULL, 10, 10, NULL, NULL }, 0 };
user_allocated_event e4 = { { 0, 0, 0, NULL, NULL, NULL, 10, 10, NULL, NULL }, 0 };
user_allocated_event e5 = { { 0, 0, 0, NULL, NULL, NULL, 0, -1, NULL, NULL }, 0 };

EXPECT_NE(0, equeue_call(&q, simple_func, &touched));
EXPECT_EQ(0, equeue_call(&q, simple_func, &touched));
EXPECT_EQ(0, equeue_call(&q, simple_func, &touched));
EXPECT_NE(0, equeue_call_every(&q, 10, simple_func, &touched));
EXPECT_EQ(0, equeue_call_every(&q, 10, simple_func, &touched));
EXPECT_EQ(0, equeue_call_every(&q, 10, simple_func, &touched));

equeue_post_user_allocated(&q, simple_func, &e1.e);
equeue_post_user_allocated(&q, simple_func, &e2.e);
equeue_post_user_allocated(&q, simple_func, &e3.e);
equeue_post_user_allocated(&q, simple_func, &e4.e);
equeue_post_user_allocated(&q, simple_func, &e5.e);
equeue_cancel_user_allocated(&q, &e3.e);
equeue_cancel_user_allocated(&q, &e3.e);

equeue_dispatch(&q, 1);
equeue_dispatch(&q, 11);

EXPECT_EQ(1, touched);
EXPECT_EQ(1, e1.touched);
@@ -1097,14 +1098,17 @@ TEST_F(TestEqueue, test_equeue_user_allocated_event_post)
EXPECT_EQ(1, e4.touched);
EXPECT_EQ(1, e5.touched);

equeue_dispatch(&q, 10);

EXPECT_EQ(1, touched);
EXPECT_EQ(1, e1.touched);
EXPECT_EQ(1, e2.touched);
EXPECT_EQ(0, e3.touched);
EXPECT_EQ(1, e4.touched);
EXPECT_EQ(1, e5.touched);
e3.e.target = 10; // set target as it's modified by equeue_call
e3.e.period = 10; // set period as it's reset by equeue_cancel
equeue_post_user_allocated(&q, simple_func, &e3.e);
equeue_dispatch(&q, 101);

EXPECT_EQ(11, touched);
EXPECT_EQ(1 , e1.touched);
EXPECT_EQ(11, e2.touched);
EXPECT_EQ(10 , e3.touched);
EXPECT_EQ(11, e4.touched);
EXPECT_EQ(1 , e5.touched);

equeue_destroy(&q);
}
17 changes: 12 additions & 5 deletions events/UserAllocatedEvent.h
View file
Original file line number Diff line number Diff line change
@@ -86,7 +86,7 @@ class UserAllocatedEvent<F, void(ArgTs...)> {
* @param f Function to execute when the event is dispatched
* @param args Arguments to bind to the callback
*/
constexpr UserAllocatedEvent(F f, ArgTs... args) : _e(get_default_equeue_event()), _c(f, args...), _equeue(), _post_ref()
constexpr UserAllocatedEvent(F f, ArgTs... args) : _e(get_default_equeue_event()), _c(f, args...), _delay(), _period(-1), _equeue(), _post_ref()
{
}

@@ -100,7 +100,7 @@ class UserAllocatedEvent<F, void(ArgTs...)> {
* @param f Function to execute when the event is dispatched
* @param args Arguments to bind to the callback
*/
constexpr UserAllocatedEvent(EventQueue *queue, F f, ArgTs... args) : _e(get_default_equeue_event()), _c(f, args...), _equeue(&queue->_equeue), _post_ref()
constexpr UserAllocatedEvent(EventQueue *queue, F f, ArgTs... args) : _e(get_default_equeue_event()), _c(f, args...), _delay(), _period(-1), _equeue(&queue->_equeue), _post_ref()
{
}

@@ -215,7 +215,7 @@ class UserAllocatedEvent<F, void(ArgTs...)> {
void delay(int delay)
{
MBED_ASSERT(!_post_ref);
equeue_event_delay(&_e + 1, delay);
_delay = delay;
}

/** Configure the period of an event
@@ -225,7 +225,7 @@ class UserAllocatedEvent<F, void(ArgTs...)> {
void period(int period)
{
MBED_ASSERT(!_post_ref);
equeue_event_period(&_e + 1, period);
_period = period;
}

/** Cancels posted event
@@ -243,14 +243,16 @@ class UserAllocatedEvent<F, void(ArgTs...)> {
*/
bool cancel()
{
return equeue_cancel_user_allocated(_equeue, &_e);
return _post_ref > 0 ? equeue_cancel_user_allocated(_equeue, &_e) : false;
}


private:
friend class EventQueue;
struct equeue_event _e;
C _c;
int _delay;
int _period;
struct equeue *_equeue;
uint8_t _post_ref;

@@ -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);
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

equeue_event_period(&_e + 1, _period);
equeue_post_user_allocated(_equeue, &EventQueue::function_call<C>, &_e);
return true;
}

bool post_on(EventQueue *queue)
{
MBED_ASSERT(queue);
if (_post_ref) {
return false;
}
_equeue = &(queue->_equeue);
core_util_atomic_incr_u8(&_post_ref, 1);
equeue_event_delay(&_e + 1, _delay);
equeue_event_period(&_e + 1, _period);
equeue_post_user_allocated(_equeue, &EventQueue::function_call<C>, &_e);
return true;
}
17 changes: 14 additions & 3 deletions events/source/equeue.c
View file
Original file line number Diff line number Diff line change
@@ -24,6 +24,12 @@
// check if the event is allocaded by user - event address is outside queues internal buffer address range
#define EQUEUE_IS_USER_ALLOCATED_EVENT(e) ((q->buffer == NULL) || ((uintptr_t)(e) < (uintptr_t)q->buffer) || ((uintptr_t)(e) > ((uintptr_t)q->slab.data)))

// for user allocated events use event id to track event state
enum {
EQUEUE_USER_ALLOCATED_EVENT_STATE_INPROGRESS = 1,
EQUEUE_USER_ALLOCATED_EVENT_STATE_DONE = 0 // event canceled or dispatching done
};

// calculate the relative-difference between absolute times while
// correctly handling overflow conditions
static inline int equeue_tickdiff(unsigned a, unsigned b)
@@ -229,7 +235,9 @@ void equeue_dealloc(equeue_t *q, void *p)
e->dtor(e + 1);
}

if (!EQUEUE_IS_USER_ALLOCATED_EVENT(e)) {
if (EQUEUE_IS_USER_ALLOCATED_EVENT(e)) {
e->id = EQUEUE_USER_ALLOCATED_EVENT_STATE_DONE;
} else {
equeue_mem_dealloc(q, e);
}
}
@@ -402,6 +410,7 @@ 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;
e->id = EQUEUE_USER_ALLOCATED_EVENT_STATE_INPROGRESS;

equeue_enqueue(q, e, tick);
equeue_sema_signal(&q->eventsema);
@@ -424,7 +433,7 @@ bool equeue_cancel(equeue_t *q, int id)

bool equeue_cancel_user_allocated(equeue_t *q, void *e)
{
if (!e) {
if (!e || ((struct equeue_event *)e)->id == EQUEUE_USER_ALLOCATED_EVENT_STATE_DONE) {
return false;
}

@@ -506,7 +515,9 @@ void equeue_dispatch(equeue_t *q, int ms)
e->target += e->period;
equeue_enqueue(q, e, equeue_tick());
} else {
equeue_incid(q, e);
if (!EQUEUE_IS_USER_ALLOCATED_EVENT(e)) {
equeue_incid(q, e);
}
equeue_dealloc(q, e + 1);
}
}