allow HWTIMER_SPEED > 1000000

[benpicco]

If HWTIMER_SPEED is greater than 1000000, division will result in 0 in HWTIMER_TICKS, HWTIMER_TICKS_TO_US

The proper solution would probably be using floating point defines to begin with, or at least casting to float for the division, but is this guaranteed to be evaluated by the compiler?

[Kijewski]

Why did you delete this macro?

[benpicco]

It isn't used anywhere

[LudwigKnuepfer]

You don't know that.

[Kijewski]

The proper solution would probably be using floating point defines to begin with

Think again. Which kind of hardware are we targeting?

[benpicco]

I was hoping it would be evauated at compile-time, I don't know wheather the preprocessor can do float to integer conversion though.

[LudwigKnuepfer]

The macro can be used on variables as well, can it not?

[N8Fear]

This macro doesn't use variables (except the all caps ones are variables which on the other hand would be against our coding conventions).

[LudwigKnuepfer]

OK, there's two questions here:

• can *SPEED use floats internally?
  • according to the first google hit (stackoverflow) the answer is NO
    • this should be double checked
• should *SPEED be float?
  • the answer is NO because we want to support hardware that has no FPU

[benpicco]

Well it's possible with boost, but I doubt whether we really want that.

[benpicco]

Looks like the simple solution does suffice though

[Kijewski]

#define HWTIMER_TICKS_TO_US(ticks) ((1000L * (ticks)) / (HWTIMER_SPEED / 1000L))

I guess this would yield a better precision. Just a gut feeling, not thoroughly tested.

[benpicco]

I tried that at first too, problem is (1000L * (ticks)) may still be less than (HWTIMER_SPEED / 1000L) or (1000L * (ticks)) overflows, I must say I didn't debug this thoroughly but just ran tests/vtimer_msg

2014-10-06 22:58:55,069 - INFO # This is thread 3
2014-10-06 22:58:55,070 - INFO # sending 1st msg
2014-10-06 22:58:55,073 - INFO # now=0:4030 -> every 2.0s: Hello World
2014-10-06 22:58:55,077 - INFO # timer_thread: set timer successfully
2014-10-06 22:58:55,078 - INFO # sending 2nd msg
2014-10-06 22:58:55,081 - INFO # now=0:8926 -> every 5.0s: This is a Test
2014-10-06 22:58:55,085 - INFO # timer_thread: set timer successfully
2014-10-06 22:58:55,086 - INFO # This is thread 4
2014-10-06 22:58:56,687 - INFO # sec=0 min=0 hour=0
2014-10-06 22:58:56,741 - INFO # sec=0 min=0 hour=0
2014-10-06 23:00:22,609 - INFO # sec=0 min=0 hour=0
2014-10-06 23:01:48,478 - INFO # sec=0 min=0 hour=0
2014-10-06 23:01:48,532 - INFO # sec=0 min=0 hour=0

I then moved zeroes around until I arrived at ((1L * ticks) / (HWTIMER_SPEED / 1000000L)) which produced a more satisfying result

2014-10-06 23:04:09,716 - INFO # This is thread 3
2014-10-06 23:04:09,717 - INFO # sending 1st msg
2014-10-06 23:04:09,720 - INFO # now=0:4031 -> every 2.0s: Hello World
2014-10-06 23:04:09,724 - INFO # timer_thread: set timer successfully
2014-10-06 23:04:09,725 - INFO # sending 2nd msg
2014-10-06 23:04:09,729 - INFO # now=0:8929 -> every 5.0s: This is a Test
2014-10-06 23:04:09,732 - INFO # timer_thread: set timer successfully
2014-10-06 23:04:09,733 - INFO # This is thread 4
2014-10-06 23:04:11,334 - INFO # sec=1 min=0 hour=0
2014-10-06 23:04:12,922 - INFO # now=2:6075 -> every 2.0s: Hello World
2014-10-06 23:04:12,926 - INFO # timer_thread: set timer successfully
2014-10-06 23:04:12,934 - INFO # sec=2 min=0 hour=0
2014-10-06 23:04:14,534 - INFO # sec=3 min=0 hour=0
2014-10-06 23:04:16,124 - INFO # now=4:7284 -> every 2.0s: Hello World
2014-10-06 23:04:16,128 - INFO # timer_thread: set timer successfully
2014-10-06 23:04:16,134 - INFO # sec=4 min=0 hour=0
2014-10-06 23:04:17,731 - INFO # now=5:11134 -> every 5.0s: This is a Test
2014-10-06 23:04:17,734 - INFO # timer_thread: set timer successfully
2014-10-06 23:04:17,736 - INFO # sec=5 min=0 hour=0
2014-10-06 23:04:19,326 - INFO # now=6:8493 -> every 2.0s: Hello World
2014-10-06 23:04:19,330 - INFO # timer_thread: set timer successfully
2014-10-06 23:04:19,336 - INFO # sec=6 min=0 hour=0
2014-10-06 23:04:20,936 - INFO # sec=7 min=0 hour=0
2014-10-06 23:04:22,528 - INFO # now=8:9702 -> every 2.0s: Hello World
2014-10-06 23:04:22,532 - INFO # timer_thread: set timer successfully
2014-10-06 23:04:22,537 - INFO # sec=8 min=0 hour=0
2014-10-06 23:04:24,137 - INFO # sec=9 min=0 hour=0
2014-10-06 23:04:25,731 - INFO # now=10:10911 -> every 2.0s: Hello World
2014-10-06 23:04:25,734 - INFO # timer_thread: set timer successfully
2014-10-06 23:04:25,737 - INFO # now=10:14228 -> every 5.0s: This is a Test
2014-10-06 23:04:25,741 - INFO # timer_thread: set timer successfully
2014-10-06 23:04:25,742 - INFO # sec=10 min=0 hour=0

(((10L * ticks) / (HWTIMER_SPEED / 100000L)) would work for the first few seconds only)

[OlegHahm]

@haukepetersen, you're the shepherd of this PR.

[haukepetersen]

I know, but I don't have a good answer/opinion so far...
Let's see for HWTIMER_TICKS(us):
case HWTIMER_SPEED > 1000000:
The new macro looks sensible, but it has one design flaw: if the value of us is close to the range of the variable, say 32-bit, an overflow is possible (but not handled).

case HWTIMER_SPEED <= 1000000:
The computation here yields very large rounding errors, if there is not an even divider. 500KHz would work fine, 750KHz for example would yield high inaccuracies... The calculation as suggested by @Kijewski would yield better results, as long as us is some orders smaller then the maximal range of it's type...

As for this PR I don't have a strong opinion. I think it does not make the situation any worse then before... So I guess merging it should be ok for now, until we have a better timer infrastructure (which is being worked on...).

[OlegHahm]

@haukepetersen, @benpicco, @Kijewski, everyone's okay to merge this?

[jnohlgard]

@haukepetersen, @benpicco Would it make sense to add other cases than only greater/less than 1000000 ? Would it be possible to avoid overflows and still have better precision then?

[haukepetersen]

ACK from my side.

@gebart it would make sense, but I would not put any thoughts to it on the current timer infrastructure. As re-thinking/designing the whole timer infrastructure is on our list, I hope the hard-coded value for 1MHz will disappear anyway...