Nut upsdev - Sep 2012 - NUT Bugs #313634 & #313714: unification & encapsulation of timer proposition

?Hello everybody,
 
I'm working on the "Use difftime for time comparison" bug
(#313634).
Charles directed me to the other one "Use monotonic clock for
monitoring"
attended to by Baruch; I believe that's very good idea, however I'd use
a bit
more encapsulated & general approach:
 
1/ I'd create an opaque timer type and its get/set/cmp/inc/dec etc interface
under common
2/ implementation of the interface would use the monotonic clock if available
(the clock_*time on Linux, potentially other monotonic clock impl. on other
systems) and time_t as a fallback
3/ when implemented, use of the timer shall be spread throughout the code,
replacing direct use of time_t
 
What do you think?
I'm also a bit reluptant to do such a change in scope of the bugfix; I'd
create
a new feature req. for that, OK?
 
Any comments/ suggestions welcome,
 
Regards,
 
Vaclav

P.S: already managed to post this from another address by mistake, so please
just discard the previous e-mail (or this one, depending on which you've
read).

--
V?clav Krpec
Software Developer
Network UPS Tools project
Eaton Opensource Team
Eaton European Innovation Center




-----------------------------
Eaton Elektrotechnika s.r.o. ~ S?dlo spolecnosti, jak je zaps?no v rejstr?ku:
Kom?rovsk? 2406, Praha 9 - Horn? Pocernice, 193 00, Cesk? Republika ~ Jm?no,
m?sto, kde byla spolecnost zaregistrov?na: Praha ~ Identifikacn? c?slo (ICO):
498 11 894
-----------------------------

My opinion on this is that it might be a little too abstract. I'd go with
what I essentially started which is to abstract just a monotonic clock with
only what is needed. If the platform doesn't have a monitonic clock than it
can be apprixmated.

This will also serve to explain the real intention of a monotonic clock and
not just a plain timer.

My 2?

Baruch
On Sep 27, 2012 1:19 PM, <VaclavKrpec at eaton.com> wrote:
> Hello everybody,
>
> I'm working on the "Use difftime for time comparison" bug
(#313634).
> Charles directed me to the other one "Use monotonic clock for
monitoring"
> attended to by Baruch; I believe that's very good idea, however I'd
use a
> bit
> more encapsulated & general approach:
>
> 1/ I'd create an opaque timer type and its get/set/cmp/inc/dec etc
> interface
> under common
> 2/ implementation of the interface would use the monotonic clock if
> available
> (the clock_*time on Linux, potentially other monotonic clock impl. on other
> systems) and time_t as a fallback
> 3/ when implemented, use of the timer shall be spread throughout the code,
> replacing direct use of time_t
>
> What do you think?
> I'm also a bit reluptant to do such a change in scope of the bugfix;
I'd
> create
> a new feature req. for that, OK?
>
> Any comments/ suggestions welcome,
>
> Regards,
>
> Vaclav
>
> P.S: already managed to post this from another address by mistake, so
> please
> just discard the previous e-mail (or this one, depending on which
you've
> read).
>
> --
> V?clav Krpec
> Software Developer
> Network UPS Tools project
> Eaton Opensource Team
> Eaton European Innovation Center
>
>
>
>
> -----------------------------
> Eaton Elektrotechnika s.r.o. ~ S?dlo spolecnosti, jak je zaps?no v
> rejstr?ku: Kom?rovsk? 2406, Praha 9 - Horn? Pocernice, 193 00, Cesk?
> Republika ~ Jm?no, m?sto, kde byla spolecnost zaregistrov?na: Praha ~
> Identifikacn? c?slo (ICO): 498 11 894
> -----------------------------
>
>
> _______________________________________________
> Nut-upsdev mailing list
> Nut-upsdev at lists.alioth.debian.org
> http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/nut-upsdev
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.alioth.debian.org/pipermail/nut-upsdev/attachments/20120927/b561a904/attachment.html>

2012/9/27 <VaclavKrpec at eaton.com>
> Hello everybody,
>
Hi Vaclav

> I'm working on the "Use difftime for time comparison" bug
(#313634).
> Charles directed me to the other one "Use monotonic clock for
monitoring"
> attended to by Baruch; I believe that's very good idea, however I'd
use a
> bit
> more encapsulated & general approach:
>
> 1/ I'd create an opaque timer type and its get/set/cmp/inc/dec etc
> interface
> under common
> 2/ implementation of the interface would use the monotonic clock if
> available
> (the clock_*time on Linux, potentially other monotonic clock impl. on other
> systems) and time_t as a fallback
> 3/ when implemented, use of the timer shall be spread throughout the code,
> replacing direct use of time_t
>
> What do you think?
> I'm also a bit reluptant to do such a change in scope of the bugfix;
I'd
> create
> a new feature req. for that, OK?
>
> Any comments/ suggestions welcome,
>
for now, Baruch's implementation seems simple and good enough.
your approach is nice (OO) but I'd like to retain this a bit more, and map
it with a more general OO/c++ discussions, later...
there are still some "time()" occurences left, here and there
(clients/upsmon.c for ex.).

I'd however like to see how it compiles, and behaves on our buildslaves.
so working on your branch, and forcing compilation is the way to go.

P.S: already managed to post this from another address by mistake, so
please
> just discard the previous e-mail (or this one, depending on which
you've
> read).
>
I added this address to the accepted senders, as for other team members...

cheers,
Arnaud
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.alioth.debian.org/pipermail/nut-upsdev/attachments/20120929/82af85d0/attachment.html>

[adding nut-upsdev back to CC list.]
> While you at it --- I?ve written the nut_clock_gettime POSIX implementation
so that
> fallback from monotonic POSIX clock impl. to RTC is actually also done if
the system
> defines _POSIX_MONOTONIC_CLOCK macro, but the call ends with EINVAL
> (i.e. not implemented) if monotonic clock is requested.
> I thought that may be a bit over-protective; as soon as the OS defines the
detection
> macro, we should be entitled to expect it to work; EINVAL should therefore
be
> considered an error and propagated.
I'm confused. Could you post pseudocode, or the actual patch, for what is
being described above?

-- 
Charles Lepple
clepple at gmail