Re: [Lcd4linux-devel] Synchronised widget updates

[Michael R. <mi...@re...>]

Hi Martin!

> To be honest, I don't... :)
Well, I think you're not alone ;-)

> First, I think we have to define the problem, because in my opinion there are two of them that need separate solving.
> One is the time-shift that is introduced by processing.  The other one is that of unsynchronised widgets, and I have
> already solved that, at least partially.  So let's talk about time-shifts.
Correct.

> First, using one-shot timers for widgets doesn't seem to make sense to me.  First, there is the overhead of deleting
> the old one and creating a new one.  Second, most (if not all) widgets are triggered regularly, so using one-shot
> timers isn't the best solution.

That's not true, at least not at all. At the moment, only two o three widgets use one-shot timers, just because the
other widgets haven't been ported to use "Properties" yet. A "property" is a widget attribute from the config file,
which is evaluated at every update (and not only once when initializing the widget). I'd like to call them "dynamic
attributes" because they may change at runtime. A (simple) example would be "update is_night()? 500 : 100"

> First, have a look at line 182, which is where a single *continuous* timer is being respawned:
> 
> } else { Timers[i].when = now; timer_inc(&Timers[i].when, Timers[i].interval); }
> 
> I think the second line (which sets the timer to "now") is plain wrong because it automatically delays a timer by the
> time the callback needs for processing.  So this line should be removed as fast as possible!

It took me some time to understand it, but you are absolutely right!

> Another, albeit smaller, problem lies in line 206:
> 
> /* delay until next timer event */ struct timeval diff; timersub(&Timers[min].when, &now, &diff);
> 
> delay->tv_sec = diff.tv_sec; /* microseconds to nanoseconds!! */ delay->tv_nsec = diff.tv_usec * 1000;
> 
> Here, the delay to the next timer event is too long, because we ignore the processing time between reading the value
> for "now" and calculating the delay.  What we need here is to update the value for "now" and only then calculate the
> delay:
> 
> /* delay until next timer event */ struct timeval diff; timersub(&Timers[min].when, &now, &diff); gettimeofday(&now,
> NULL);


Again, you are right.

> So it seems that we don't need any "processing magic" at all.  Just convert the widgets to continuous timers and make
> the two changes in "timer.c", and we're set!  Actually, I didn't think it was *that* easy, and maybe I have overseen
> something -- but maybe that's just it! :)

Great!

So the timers should work correctly now, and there should be no mor time-shifts, right?


And now for your 2nd mail:

> Moreover, I have found out that you don't even have to change any single line in the widget code if you apply the
> patch for synchronising widgets I have sent before.  I have applied this patch, and now the widgets are synced (at
> least within their respective groups) and the timers are correct to the microsecond!

I'm afraid I did miss that patch you are mentioning... wasn't it that "timer_group" stuff? Isn't it necessary to replace
the timer_add() call in the widget code?


> I repeat my old question: shall I commit, or shan't I commit?  ;)

I can't answer because... (see above)

but: if there is no more timeshift, how can widgets get unsync?

OTOH: please consider two things: the "initial delay" and the "properties" stuff from above.

please, please, don't get me wrong: I in no way dislike your patch. I just want to solve it for *all* timers, not only
widget updates.



bye, Michael

-- 
Michael Reinelt <mi...@re...>
http://home.pages.at/reinelt
GPG-Key 0xDF13BA50
ICQ #288386781