>> >> diff --git a/example/timer/odp_timer_test.c
>> >> b/example/timer/odp_timer_test.c
>> >
>> >
>> > We need to rename this, having a "test" in example makes no sense at
>> > all.
>> > This patch series should rename this as an example - see below on the
>> > implication on the large amount of code stored as code in the header
>> > file
>> > below, probably that needs to be in the resulting example.
>> OK. But I wanted to limit the changes introduced by this patch series
>> and then post additional patches that clean up the testing of the
>> timer (e.g. odp_timer_ping has to go).
>
>
> Can we capture the work that would still be needed in here 1.0
> implementation deltas doc ?
> I am worried you will skip the country :), but if you can do this soon, in
> the interests of progress I'd be ok with the list in the doc
Yes, I have added some text on odp_timer_test and odp_timer_ping.
>
>>
>>
>> > The "test" portion of this should become a cunit test
>> OK. But perhaps not in this patch.
>>
>
> Can we capture the work that would still be needed in here 1.0
> implementation deltas doc ?
Ditto.
>> > Use C comments /* */
>> I can't because this is inside another C comment and those are not
>> recursive.
>
>
> I had no problem with doxygen or the compiler that I noticed when I applied
> the patch can we take a look ?
Take a look at what?
>
>>
>>
>> > Can this example not be linked to the actual example so that the example
>> > in
>> > the doc is always assured to be as correct as the tests executing it in
>> > CI ?
>> These are not complete examples so cannot compile. I just want to show
>> how the API is supposed to be used for a number of common use cases.
>> Possibly a good example will be able to replace all of these header
>> file/doxygen fragments. Would that be better?
>
>
> Striving for the best case
>
> A good example which runs and that allows doxygen to pull chunks of code out
> to to display would be best.
> If the code can just compile that might be a better assurance that they
> don't rot in the header file.
>
> But I would stop short of verbatim including it from another file with no
> processing, it may as stay as it is and we can check back after 2.0 to see
> if the examples still work :)
Yes.
>
>
>
>>
>>
>>
>> > After reviewing further there are a lot of examples in the header file I
>> > am
>> > sure they will rot unless we link them to the actual example code.
>> They will rot only if the API is changed (except pure extensions). But
>> code that actually compiles against the header file is of course
>> better.
>>
>>
>> >
>> >>
>> >> +#define SEC 1000000000ULL //1s expressed in nanoseconds
>> >> +odp_timer_pool_t tcp_tpid =
>> >> + odp_timer_pool_create("TCP",
>> >> + buffer_pool,
>> >> + 1000000,//resolution 1ms
>> >> + 0,//min tmo
>> >> + 7200 * SEC,//max tmo length 2hours
>> >> + 40000,//num_timers
>> >> + true,//shared
>> >> + ODP_CLOCK_CPU
>> >> + );
>> >> +if (tcp_tpid == ODP_TIMER_POOL_INVALID)
>> >> +{
>> >> + //Failed to create timer pool => fatal error
>> >> +}
>> >> +
>> >> +
>> >> +//Setting up a new connection
>> >> +//Allocate retransmission timeout (identical for supervision timeout)
>> >> +//The user pointer points back to the connection context
>> >> +conn->ret_tim = odp_timer_alloc(tcp_tpid, queue, conn);
>> >> +//Check if all resources were successfully allocated
>> >> +if (conn->ret_tim == ODP_TIMER_INVALID)
>> >> +{
>> >> + //Failed to allocate all resources for connection => tear down
>> >> + //Tear down connection
>> >> + ...
>> >> + return false;
>> >> +}
>> >> +//All necessary resources successfully allocated
>> >> +//Compute initial retransmission length in timer ticks
>> >> +conn->ret_len = odp_timer_ns_to_tick(tcp_tpid, 3 * SEC);//Per RFC1122
>> >> +//Allocate a timeout buffer
>> >> +conn->tmo_buf = odp_buffer_alloc(buffer_pool);
>> >> +if (conn->tmo_buf == ODP_BUFFER_INVALID)
>> >> + ODP_ABORT("Failed to allocate timeout buffer\n");
>> >> +//Arm the timer with our timeout
>> >> +conn->expected = odp_timer_set_rel(conn->ret_tim, conn->ret_len,
>> >> + &conn->tmo_buf);
>> >> +//Check return value for tooearly or toolate expiration tick
>> >
>> >
>> > too early and too late needs spaces - multiple instances
>> I was mimicking the symbolic error codes.
>> From odp_timer.h:
>> #define ODP_TICK_TOOEARLY 0xFFFFFFFFFFFFFFFDULL
>> #define ODP_TICK_TOOLATE 0xFFFFFFFFFFFFFFFEULL
>>
>
> Ok, if there is a good reason, it just looked funny to my eye.
It's a reason, it's subjective whether it's a good reason. But I our
public identifiers to be constructed by three parts. First the "odp_"
prefix. Second some kind of module or context identifier, e.g. "TICK"
here. And third the specific part, either what the function does or
the error code etc.
ODP_TICK_TOOEARLY vs. ODP_TICK_TOO_EARLY
ODP_TICK_TOOLATE vs. ODP_TICK_TOO_LATE
?
>
>>
>> Do you require additional underscores here as well?
>
>
> I think we should use underscores consistently, we really dont across the
> code now and I am not sure what the rule is for when we don't use them, if
> anyone knows I'll add it to the API guide lines doc.
>
> I would fix the #defines too.
OK
_______________________________________________
lng-odp mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/lng-odp
