On Wed, Jun 8, 2016 at 1:57 AM, Maxim Uvarov <maxim.uva...@linaro.org> wrote:
> On 06/08/16 06:07, Bill Fischofer wrote: > >> >> >> On Tue, Jun 7, 2016 at 3:46 PM, Maxim Uvarov <maxim.uva...@linaro.org >> <mailto:maxim.uva...@linaro.org>> wrote: >> >> Example for simple timer usage. >> https://bugs.linaro.org/show_bug.cgi?id=2090 >> >> Signed-off-by: Maxim Uvarov <maxim.uva...@linaro.org >> <mailto:maxim.uva...@linaro.org>> >> >> --- >> example/timer/.gitignore | 1 + >> example/timer/Makefile.am | 12 ++- >> example/timer/odp_timer_simple.c | 167 >> +++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 177 insertions(+), 3 deletions(-) >> create mode 100644 example/timer/odp_timer_simple.c >> >> diff --git a/example/timer/.gitignore b/example/timer/.gitignore >> index eb59265..f53a0df 100644 >> --- a/example/timer/.gitignore >> +++ b/example/timer/.gitignore >> @@ -1 +1,2 @@ >> odp_timer_test >> +odp_timer_simple >> diff --git a/example/timer/Makefile.am b/example/timer/Makefile.am >> index fcb67a9..1c733d3 100644 >> --- a/example/timer/Makefile.am >> +++ b/example/timer/Makefile.am >> @@ -1,10 +1,16 @@ >> include $(top_srcdir)/example/Makefile.inc >> >> -bin_PROGRAMS = odp_timer_test$(EXEEXT) >> +bin_PROGRAMS = odp_timer_test$(EXEEXT) \ >> + odp_timer_simple$(EXEEXT) >> odp_timer_test_LDFLAGS = $(AM_LDFLAGS) -static >> odp_timer_test_CFLAGS = $(AM_CFLAGS) -I${top_srcdir}/example >> +dist_odp_timer_test_SOURCES = odp_timer_test.c >> + >> +odp_timer_simple_LDFLAGS = $(AM_LDFLAGS) -static >> +odp_timer_simple_CFLAGS = $(AM_CFLAGS) -I${top_srcdir}/example >> +dist_odp_timer_simple_SOURCES = odp_timer_simple.c >> + >> +TESTS = odp_timer_simple >> >> noinst_HEADERS = \ >> $(top_srcdir)/example/example_debug.h >> - >> -dist_odp_timer_test_SOURCES = odp_timer_test.c >> diff --git a/example/timer/odp_timer_simple.c >> b/example/timer/odp_timer_simple.c >> new file mode 100644 >> index 0000000..d4917c3 >> --- /dev/null >> +++ b/example/timer/odp_timer_simple.c >> @@ -0,0 +1,167 @@ >> +/* Copyright (c) 2016, Linaro Limited >> + * All rights reserved. >> + * >> + * SPDX-License-Identifier: BSD-3-Clause >> + */ >> +/** >> + * @file >> + * >> + * @example odp_timer_simple.c ODP simple example to schedule timer >> + * action for 1 second. >> + */ >> + >> +#include <string.h> >> +#include <stdlib.h> >> +#include <example_debug.h> >> + >> +/* ODP main header */ >> +#include <odp_api.h> >> + >> +int main(int argc ODP_UNUSED, char *argv[] ODP_UNUSED) >> +{ >> + odp_instance_t instance; >> + odp_pool_param_t params; >> + odp_timer_pool_param_t tparams; >> + odp_pool_t tmo_pool; >> + odp_timer_pool_t timer_pool; >> + odp_queue_param_t qparam; >> + odp_queue_t queue; >> + odp_event_t ev = ODP_EVENT_INVALID; >> + odp_timer_t tim; >> + uint64_t sched_tmo; >> + int i, rc; >> + uint64_t period; >> + uint64_t tick; >> + odp_timeout_t tmo; >> + int ret = 0; >> + >> + /* >> + * Init ODP app >> + */ >> + if (odp_init_global(&instance, NULL, NULL)) >> + goto err_global; >> + >> + if (odp_init_local(instance, ODP_THREAD_CONTROL)) >> + goto err_local; >> + >> + /* >> + * Create pool for timeouts >> + */ >> + odp_pool_param_init(¶ms); >> + params.tmo.num = 10; >> + params.type = ODP_POOL_TIMEOUT; >> + >> + tmo_pool = odp_pool_create("msg_pool", ¶ms); >> >> >> Is "msg_pool" the best choice of name for a timeout pool? "timeout_pool" >> would seem to make more sense here. >> > > agree, will change. > > > + if (tmo_pool == ODP_POOL_INVALID) { >> + ret += 1; >> + goto err_tp; >> + } >> + >> + /* >> + * Create pool of timeouts >> + */ >> + tparams.res_ns = 10 * ODP_TIME_USEC_IN_NS; >> + tparams.min_tmo = 10 * ODP_TIME_USEC_IN_NS; >> + tparams.max_tmo = 1 * ODP_TIME_SEC_IN_NS; >> + tparams.num_timers = 1; /* One timer per worker */ >> + tparams.priv = 0; /* Shared */ >> + tparams.clk_src = ODP_CLOCK_CPU; >> + timer_pool = odp_timer_pool_create("timer_pool", &tparams); >> + if (timer_pool == ODP_TIMER_POOL_INVALID) { >> + ret += 1; >> + goto err; >> + } >> + >> + /* >> + * Create a queue for timer test >> + */ >> + odp_queue_param_init(&qparam); >> + qparam.type = ODP_QUEUE_TYPE_SCHED; >> + qparam.sched.prio = ODP_SCHED_PRIO_DEFAULT; >> + qparam.sched.sync = ODP_SCHED_SYNC_PARALLEL; >> + qparam.sched.group = ODP_SCHED_GROUP_ALL; >> + >> + queue = odp_queue_create("timer_queue", &qparam); >> + if (queue == ODP_QUEUE_INVALID) { >> + ret += 1; >> + goto err; >> + } >> + >> + tim = odp_timer_alloc(timer_pool, queue, NULL); >> + if (tim == ODP_TIMER_INVALID) { >> + EXAMPLE_ERR("Failed to allocate timer\n"); >> + ret += 1; >> + goto err; >> + } >> + >> + tmo = odp_timeout_alloc(tmo_pool); >> + if (tmo == ODP_TIMEOUT_INVALID) { >> + EXAMPLE_ERR("Failed to allocate timeout\n"); >> + return -1; >> + } >> + >> + ev = odp_timeout_to_event(tmo); >> + >> + /* Calculate period for timer in uint64_t value, in >> current case >> + * we will schedule timer for 1 second */ >> + period = odp_timer_ns_to_tick(timer_pool, 1 * >> ODP_TIME_SEC_IN_NS); >> + >> + /* Get current tick uint64_t value */ >> + tick = odp_timer_current_tick(timer_pool); >> + >> + rc = odp_timer_set_abs(tim, tick + period, &ev); >> + if (odp_unlikely(rc != ODP_TIMER_SUCCESS)) { >> + /* Too early or too late timeout requested */ >> + EXAMPLE_ABORT("odp_timer_set_abs() failed: %d\n", >> + rc); >> + } >> + >> + /* Wait time to return from odp_schedule() if there are no >> + * events >> + */ >> + sched_tmo = odp_schedule_wait_time(2 * ODP_TIME_SEC_IN_NS); >> + >> + for (i = 0; i < 5; i++) { >> + odp_time_t time; >> + >> + /* Program timeout action on current tick + period */ >> + tick = odp_timer_current_tick(timer_pool); >> + rc = odp_timer_set_abs(tim, tick + period, &ev); >> >> >> This replaces the odp_timer_set_abs() call made prior to entering this >> loop. Is that intentional? If yes, a comment would seem appropriate to >> explain why. >> > > it has to be only inside this loop. Above loop is not needed, will remove. > > + /* Too early or too late timeout requested */ >> + if (odp_unlikely(rc != ODP_TIMER_SUCCESS)) >> + EXAMPLE_ABORT("odp_timer_set_abs() failed: >> %d\n", >> + rc); >> + >> + /* Wait for 2 seconds for timeout action to be >> generated */ >> + ev = odp_schedule(&queue, sched_tmo); >> + if (ev == ODP_EVENT_INVALID) >> + EXAMPLE_ABORT("Invalid event\n"); >> >> >> Isn't the real error here that the 1 second timer failed to trigger after >> the scheduler waited for 2 seconds? >> > Yes, that is real error if there was no timeout. > > > > + if (odp_event_type(ev) != ODP_EVENT_TIMEOUT) >> + EXAMPLE_ABORT("Unexpected event type (%u) >> received\n", >> + odp_event_type(ev)); >> + >> + time = odp_time_global(); >> + printf("timer tick %d, time ns %" PRIu64 "\n", >> + i, odp_time_to_ns(time)); >> + >> + /* Do not free current event, just go back to loop >> and program >> + * timeout to next second. >> + */ >> + } >> + >> + /* Destroy created resources */ >> + odp_timer_cancel(tim, &ev); >> + odp_timer_free(tim); >> + odp_event_free(ev); >> >> >> Since examples illustrate best practices, shouldn't RCs be checked for >> these calls? >> > > odp_event_free() is void. For 2 above function I will add ret check. > > > + >> + ret += odp_queue_destroy(queue); >> +err: >> + odp_timer_pool_destroy(timer_pool); >> >> >> Why not ret += for consistency with the rest of these exit calls? >> Oversight? >> > > Because it's void function in api. For me it looks also not really > consistent to other calls. > I should have checked. You're right, that does seem inconsistent. Perhaps something to revisit in Tiger Moth? > > So I will send v2 with above comments. > > Maxim. > >> >> +err_tp: >> + ret += odp_pool_destroy(tmo_pool); >> + ret += odp_term_local(); >> +err_local: >> + ret += odp_term_global(instance); >> +err_global: >> + return ret; >> +} >> -- >> 2.7.1.250.gff4ea60 >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> >> https://lists.linaro.org/mailman/listinfo/lng-odp >> >> >> > _______________________________________________ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
