Inversion of timer.*() parameters

[Martin Guy]

In the recent "system timer" mods, an additional change was made that
inverst the order of the timer parameters from

    tmr.delay(timer_id, delay)
to
    tmr.delay(delay, timer_id)

and similarly for the other tmr.*() functions

Please let us think again about this change, about which there was no
discussion that I saw.

The very small advantage of it is to be able to write
     tmr.delay(100)
instead of
     tmr.delay(tmr.SYS_TIMER, 100)
which is just syntactic sugar with no operational advantage.

The disavantages are
- it breaks almost all code ever written for eLua
- it makes it impossible to write code that works in different
versions of eLua, unless you do all delays with for loops tuned to the
CPU speed and use timers for nothing else, since there is no sane way
to find out which syntax is being used.
- all existing firmware that we have produced and shipped would need
changing if we decide to follow this change, and all the alternate
firmware versions we have published become junk
- people writing eLua video tutorials cannot speak about timers unless
they include a section on changing the syntax according to which
version of eLua you are using, or resign themselves to it not working
in all existing (or all future) boards

Code that users send me asking me to help resolve bugs now contains stuff like:

                if new_elua then

                        tmr.delay( delay_time, timer_id )

                else

                        tmr.delay( timer_id, delay_time )

                end

(this is an example from today!). Before this sort of thing becomes
commonplace, or before people resort to inventing tricks to discover
which is the syntax is today's firmware, PLEASE can we go back to the
original parameter order?

As a compromise we could let tmr.delay(nil, 100) to mean "use the
system timer", which is less ugly without breaking everything.

I also notice that three new system variables tmr.SYS_TIMER,
uart.SYS_TIMER and net.SYS_TIMER have appeared.  What are the second
two for?

Cheers

    M
_______________________________________________
eLua-dev mailing list
[hidden email]
https://lists.berlios.de/mailman/listinfo/elua-dev

[jbsnyder]

Up front comment: It would be really great if we could get opinions
from not just people with commit access on this one, feel free to
weigh in here even if it's just a simple yay or nay type of response.

On Thu, Dec 29, 2011 at 2:09 PM, Martin Guy <[hidden email]> wrote:

> In the recent "system timer" mods, an additional change was made that
> inverst the order of the timer parameters from
>
>    tmr.delay(timer_id, delay)
> to
>    tmr.delay(delay, timer_id)
>
> and similarly for the other tmr.*() functions
>
> Please let us think again about this change, about which there was no
> discussion that I saw.

This is true, and changes like this in trunk have often lead by
"here's working code" that gets merged in rather than having drawn out
discussions.  That said, I think whatever opinions people may hold
we're not feeling bound to one approach or the other and switching to
a new one should be done on merits as well as the fact that there's
already an working implementation.

>
> The very small advantage of it is to be able to write
>     tmr.delay(100)
> instead of
>     tmr.delay(tmr.SYS_TIMER, 100)
> which is just syntactic sugar with no operational advantage.

I think the suggested operational advantage was to discourage the use
of individual timers for every last hardware or software component
that needed some type of timing service, with the idea being that in
90-some-ercent of cases people would be typing tmr.delay( delay_time )
with no timer being specified.

Yes, it's also just syntactic sugar.

>
> The disavantages are
> - it breaks almost all code ever written for eLua
> - it makes it impossible to write code that works in different
> versions of eLua, unless you do all delays with for loops tuned to the
> CPU speed and use timers for nothing else, since there is no sane way
> to find out which syntax is being used.
> - all existing firmware that we have produced and shipped would need
> changing if we decide to follow this change, and all the alternate
> firmware versions we have published become junk
> - people writing eLua video tutorials cannot speak about timers unless
> they include a section on changing the syntax according to which
> version of eLua you are using, or resign themselves to it not working
> in all existing (or all future) boards
>
> Code that users send me asking me to help resolve bugs now contains stuff like:
>
>                if new_elua then
>
>                        tmr.delay( delay_time, timer_id )
>
>                else
>
>                        tmr.delay( timer_id, delay_time )
>
>                end
>
> (this is an example from today!). Before this sort of thing becomes
> commonplace, or before people resort to inventing tricks to discover
> which is the syntax is today's firmware, PLEASE can we go back to the
> original parameter order?

My biggest issue with the ordering change is not even so much that
it's a reordering of parameters, but that it differs from the behavior
of every other module that consumes a hardware resource, which puts
the resource first followed by the parameter(s) to be applied to it.
Also, I rather don't like the ordering for tmr.gettimediff: end,
start, [id]

>
> As a compromise we could let tmr.delay(nil, 100) to mean "use the
> system timer", which is less ugly without breaking everything.

That's perhaps not a bad option.  I also wouldn't mind the approach I
mentioned in an earlier message which was essentially to provide
wrappers to use the system timer:
tmr.sys.delay( delay_time) == tmr.delay(tmr.SYS_TIMER, delay_time )
etc..

It might be a tad slower on each call, but it would probably be pretty
space efficient to handle the difference with a metamethod or two.

I'd be fine with this or the 'nil' id approach.

>
> I also notice that three new system variables tmr.SYS_TIMER,
> uart.SYS_TIMER and net.SYS_TIMER have appeared.  What are the second
> two for?

Not sure.

-jsnyder
_______________________________________________
eLua-dev mailing list
[hidden email]
https://lists.berlios.de/mailman/listinfo/elua-dev

[BogdanM]

Hi,

(quick reply, I'm on vacation and on my ipad)

On Thursday, December 29, 2011, Martin Guy <[hidden email]> wrote:
> In the recent "system timer" mods, an additional change was made that
> inverst the order of the timer parameters from
>
>    tmr.delay(timer_id, delay)
> to
>    tmr.delay(delay, timer_id)
>
> and similarly for the other tmr.*() functions
>
> Please let us think again about this change, about which there was no
> discussion that I saw.
>
> The very small advantage of it is to be able to write
>     tmr.delay(100)
> instead of
>     tmr.delay(tmr.SYS_TIMER, 100)
> which is just syntactic sugar with no operational advantage.

The main idea was definitely NOT syntax sugar. It was that the user shouldn't have to worry about timer IDs in the first place (and that this was a big problem with the original design) except for some rare cases. This philosophy should also be reflected in the API.

>
> The disavantages are
> - it breaks almost all code ever written for eLua

which isn't that much code yet. I shoud have made it explicit somewhere that API changes are to EXPECTED until 1.0. actually i will add this to the docs.

> - it makes it impossible to write code that works in different
> versions of eLua, unless you do all delays with for loops tuned to the
> CPU speed and use timers for nothing else, since there is no sane way
> to find out which syntax is being used.
> - all existing firmware that we have produced and shipped would need
> changing if we decide to follow this change, and all the alternate
> firmware versions we have published become junk

Obsolete syntax/modules/functions are a reality in a lot of software projects. They exist now and they will continue to exist in elua in the future. The solution is not to forget about them, just to find a better way to handle them (see below).

> - people writing eLua video tutorials cannot speak about timers unless
> they include a section on changing the syntax according to which
> version of eLua you are using, or resign themselves to it not working
> in all existing (or all future) boards

True. Patrick would agree with you wholehearteadly :)

>
> Code that users send me asking me to help resolve bugs now contains stuff like:
>
>                if new_elua then
>
>                        tmr.delay( delay_time, timer_id )
>
>                else
>
>                        tmr.delay( timer_id, delay_time )
>
>                end
>
> (this is an example from today!). Before this sort of thing becomes
> commonplace, or before people resort to inventing tricks to discover
> which is the syntax is today's firmware, PLEASE can we go back to the
> original parameter order?

As far as I am concerned, the answer is no. I simply find the new way much more natural and logical.

>
> As a compromise we could let tmr.delay(nil, 100) to mean "use the
> system timer", which is less ugly without breaking everything.

As always, beauty is in the eye of the beholder. for me, tmr.delay(nil,100) is ugly as sin and leaves anybody not familiar with the API wondering "WTF does that mean?" tmr.delay(100) is much more clear and descriptive.

>
> I also notice that three new system variables tmr.SYS_TIMER,
> uart.SYS_TIMER and net.SYS_TIMER have appeared.  What are the second
> two for?

just for completion, they all reffer to the system timer ID. I understand why they can be confusing, there's no problem if we remove them.

conclusion: i believe API changes are both necessarry and unavoidable. We can make the API transition smoother by designing a deprecation mechanism. The users will be able to use the old API as usual for a while (for example until the next official release) with a warning from eLua, being able to experiment with the new API at the same time. The old API will still be there for a number of versions, but under a different name, then it will disappear completely.
An example for the timer module:

tmr.delay(tmr.SYS_TIMER, 100) --current version
future.tmr.delay(100) -- this is going to be used in the future

(this mirrors partially the idea of futures found in Python)

Or:

local tmr=tmr -- to use the old version
local tmr= future.tmr -- to use the new version

I'm making this up as i write, so i'm sure it has lots of drawbacks (in particular, i'd be very happy if we can avoid module versioning) and can be improved a lot. I'm opened to suggestions in this area.

Bogdan

>
> Cheers
>
>    M
> _______________________________________________
> eLua-dev mailing list
> [hidden email]
> https://lists.berlios.de/mailman/listinfo/elua-dev
>
_______________________________________________
eLua-dev mailing list
[hidden email]
https://lists.berlios.de/mailman/listinfo/elua-dev

[Martin Guy]

On 29 December 2011 22:54, Bogdan Marinescu <[hidden email]> wrote:
> On Thursday, December 29, 2011, Martin Guy <[hidden email]> wrote:
>> In the recent "system timer" mods, an additional change was made that
>> inverst the order of the timer parameters from
>>
>>    tmr.delay(timer_id, delay)
>> to
>>    tmr.delay(delay, timer_id)

> API changes are to EXPECTED until 1.0. actually i will add this to the docs.

That's fine - how else would you improve and extend the user interface
except by changing it?
But when changes can be made in a way that improves functionality
without breaking existing code, so much the better for anyone using
it.  If for some reason you must make a non-backward-compatible
change, so be it. I don't see any "must" here. The system timer is
great, but can be made available without creating an incompatibility.

>> As a compromise we could let tmr.delay(nil, 100) to mean "use the system timer"

I dunno. That fits in with the existing structure, where physical
timers and virtual timers coexisted happily.  If people are boggled by
tmr.delay(nil, 100), all they have to do is read the manual, and will
find out about the extra things they can do with hardware timers and
virtual timers.

Another option would be to implement something new such as timer.*()
for the new interface, which can redefine the primitives in the light
of the evolution of the timer construct.
After all, the platform interface has not undergone an incompatible
change, only the Lua layer.

> tmr.delay(tmr.SYS_TIMER, 100) --current version
> future.tmr.delay(100) -- this is going to be used in the future

I'm not so sure about that, since anything written to use
future.tmr.*() would stop working when future.tmr becomes tmr, so we
are still create a fracture between code for different versions of
eLua, which cause long-term pain.

   M
_______________________________________________
eLua-dev mailing list
[hidden email]
https://lists.berlios.de/mailman/listinfo/elua-dev

[jbsnyder]

On Sat, Jan 7, 2012 at 5:23 AM, Martin Guy <[hidden email]> wrote:

> On 29 December 2011 22:54, Bogdan Marinescu <[hidden email]> wrote:
>> On Thursday, December 29, 2011, Martin Guy <[hidden email]> wrote:
>>> In the recent "system timer" mods, an additional change was made that
>>> inverst the order of the timer parameters from
>>>
>>>    tmr.delay(timer_id, delay)
>>> to
>>>    tmr.delay(delay, timer_id)
>
>> API changes are to EXPECTED until 1.0. actually i will add this to the docs.
>
> That's fine - how else would you improve and extend the user interface
> except by changing it?
> But when changes can be made in a way that improves functionality
> without breaking existing code, so much the better for anyone using
> it.  If for some reason you must make a non-backward-compatible
> change, so be it. I don't see any "must" here. The system timer is
> great, but can be made available without creating an incompatibility.
>
>>> As a compromise we could let tmr.delay(nil, 100) to mean "use the system timer"
>
> I dunno. That fits in with the existing structure, where physical
> timers and virtual timers coexisted happily.  If people are boggled by
> tmr.delay(nil, 100), all they have to do is read the manual, and will
> find out about the extra things they can do with hardware timers and
> virtual timers.

I think he was making an "ugliness" argument against this.  Which I do
and don't buy.  I've seen this sort of thing done for other languages
like MATLAB where there aren't keyword parameters (as in python).  In
MATLAB optional parameters that aren't the last parameter can be
passed as empty lists, like this:
y = filter(b,a,X,zi,dim)
y = filter(b,a,X,[],dim)
http://www.mathworks.com/help/techdoc/ref/filter.html

The MATLAB approach can get rather ugly though because sometimes you
end up calling functions in this manner if the defaults are fine:
foo(x,y,[],[],[],fs)

'nil' however is not so bad, "Which of the system timers do you want?"
nil or none, OK you get a system timer (or a default timer)

>
> Another option would be to implement something new such as timer.*()
> for the new interface, which can redefine the primitives in the light
> of the evolution of the timer construct.

Something like this sounds appealing.  Or, the wrapper function idea I
suggested where system timer functionality could be accessed by using:
tmr.sys.*

Likewise if there's concern about maybe doing consistent namespaces
with that you could have a deprecation of tmr.* -> tmr.hw.*?

> After all, the platform interface has not undergone an incompatible
> change, only the Lua layer.

Well, the platform layer did make some changes, some of which I think
were incompatible, like ordering of diff parameters.

>
>> tmr.delay(tmr.SYS_TIMER, 100) --current version
>> future.tmr.delay(100) -- this is going to be used in the future
>
> I'm not so sure about that, since anything written to use
> future.tmr.*() would stop working when future.tmr becomes tmr, so we
> are still create a fracture between code for different versions of
> eLua, which cause long-term pain.

Well, on the plus side of this sort of approach, it might make it
easier to identify what notation the code is using, which would make
it easier to do things like automatically rewrite it in codebases (ala
python 2to3: http://wiki.python.org/moin/2to3), and to have explicit
failure messages once the deprecated style gets dropped.

That said, if we were going to go this approach I wouldn't put it in a
namespace like "future" I would prefer something like dividing tmr
into tmr.sys.* and tmr.hw.* or something like that and have the
default point alias functions on tmr.hw.* onto tmr.* for the short
term.


The one thing I really want to change is the ordering of the diff
parameters:  start should go before end :-)


>
>   M
> _______________________________________________
> eLua-dev mailing list
> [hidden email]
> https://lists.berlios.de/mailman/listinfo/elua-dev
_______________________________________________
eLua-dev mailing list
[hidden email]
https://lists.berlios.de/mailman/listinfo/elua-dev