[Tiva] delayMicroseconds unreliable

[spirilis 1,265]

Oh wait, bet the cc3200 is using SysTick wholly for millis() too.

 

Sent from my Galaxy Note II with Tapatalk 4

[pabigot 355]

I'm not paying a lot of attention to this because it's Energia and Tiva neither of which I use right now, but for low-overhead delays of reasonable duration you might want to look at how BSPACM does it. For microsecond counts you should just be able to convert the count to cycles based on the CPU frequency. Wouldn't be dependent on systick then.

 

There's some discussion of this on the Stellarisiti forums somewhere.

 

For what that's worth.

[spirilis 1,265]

Thanks @@pabigot ! Always interested to see how you do these things :-)  I am going to peek at your techniques for low-power stuff soon too, as that's something I want to add for all the supported ARM platforms (we have a decent system in place for low-power sleeps on MSP430).

[dpharris 13]

 

These are confusing me a bit-

        // Systick timer rolls over every 1000000/SYSTICKHZ microseconds 

        if (us > 1000000UL/SYSTICKHZ/10*8) {

                delay(us / 1000);  // delay milliseconds

 

If I'm not mistaken, that means if us > 125 (1000000 / 100 / 80) then it tries to do a delay(125/1000), or a delay(0) in other words.  I think I'll just make that if (us > 999) since SYSTICKHZ is 100 anyhow (every 10ms).

edit: er no, that would cause an infinite loop, so it's if (us > 9999) ... that should work.

 

 

I read that calculation as:  (1000000 / 100 / 10) * 8 and not (1000000 / 100 / (10*8), a factor of 64

 

David

[spirilis 1,265]

I read that calculation as:  (1000000 / 100 / 10) * 8 and not (1000000 / 100 / (10*8), a factor of 64

 

David

Ah yeah, hmm, that makes more sense :-)  Still very much arbitrary though IMO, so I put in a programmatic alternative.

[igor 163]

These are confusing me a bit-

        // Systick timer rolls over every 1000000/SYSTICKHZ microseconds 

        if (us > 1000000UL/SYSTICKHZ/10*8) {

                delay(us / 1000);  // delay milliseconds

 

If I'm not mistaken, that means if us > 125 (1000000 / 100 / 80) then it tries to do a delay(125/1000), or a delay(0) in other words.  I think I'll just make that if (us > 999) since SYSTICKHZ is 100 anyhow (every 10ms).

edit: er no, that would cause an infinite loop, so it's if (us > 9999) ... that should work.

 

Or programmatically... 1000000UL / SYSTICKHZ - 1

 

Looks pretty good now, issued the pullreq.

 

The intention here was to come up with a value close to but less than 1000000UL / SYSTICKHZ 

In this case I was aiming for 0.8 * 1000000UL / SYSTICKHZ (8000, or 8 milliseconds, as noted above).

 

At least in Energia 13, the result of 1000000UL/SYSTICKHZ/10*8 is the same as 8 * 1000000UL/SYSTICKHZ/10 or 8000

I thought that was pretty standard for how most programming languages deal with division vs. multiplication, (i.e. I didn't think about it being unclear) but doing a little search I see the interpretation of such an expression outside of programming languages has given rise to some discussion and a variety of interpretations are in common use.  So it is something one should avoid writing.

 

No major reason for choosing 0.8 (could be anything close to 1).

I wrote it as a fraction rather than a decimal just to avoid having a float in there (sure it is all done by the compiler, but I still avoid floats where don't need them).

Subtracting a value from the limiting value works fine too.  (Well there were reasons why I tend to go for multiply by a fraction in such cases, but enough said.)

 

Sorry for the confusion.

[igor 163]

I'm not paying a lot of attention to this because it's Energia and Tiva neither of which I use right now, but for low-overhead delays of reasonable duration you might want to look at how BSPACM does it. For microsecond counts you should just be able to convert the count to cycles based on the CPU frequency. Wouldn't be dependent on systick then.

 

There's some discussion of this on the Stellarisiti forums somewhere.

 

For what that's worth.

 

http://forum.stellarisiti.com/topic/1908-execution-time-the-easy-way

http://forum.stellarisiti.com/topic/1980-delay-cycles/

(Thanks @@pabigot for the reference on TimerWrap - it was in the process of trying to really understand that that I stumbled on the problem with the SysTick delay here.)

 

I had thought about using the cycle counter as another way to test the delay function.

[igor 163]

Oh wait, bet the cc3200 is using SysTick wholly for millis() too.

 

Sent from my Galaxy Note II with Tapatalk 4

 

Yes, it does.

 

Setting SYSTICKHZ to 1000 on the cc3200 would make the argument to SysTickCbFuncs(ui32TimeMS) consistent with its name.

[spirilis 1,265]

@@igor Ok that all makes more sense now... the 8 threw me off because I thought it was a hardcoded assumption based on the TM4C123's 80MHz clock, then my math error threw me off even further :-)

 

Sent from my Galaxy Note II with Tapatalk 4

[spirilis 1,265]

@@igor Thanks again for your code & help, what I have done is changed tack a bit - I got the CC3200 core working fantastically, since it's already implemented as a pure-SysTick implementation, and now I am going to backport to Tiva so we can remove the TIMER5 requirement once and for all.  I'll let you know when that's ready to go

 

Also micros() was mixed up on cc3200, as it used SysTickValueGet() instead of (SysTickPeriodGet, aka F_CPU / SYSTICKHZ) - SysTickValueGet()...

[spirilis 1,265]

@@energia is going to review the pull request before merging, but for now I made a zip "service pack" with all the recent changes from github issue #475.

E13_servicepack_timers_10272014.zip

 

Changes:

- cc3200's SysTick-based implementation fixed for delays and micros, and its implementation backported to lm4f

- lm4f (Stellaris/Tiva) now uses SysTick exclusively for millis, micros, delays; Timer5 has been relieved of service.

- cc3200 now has sleep, sleepSeconds, suspend and wakeup just like lm4f; it's the same (strawman, non-low-power) implementation.  I want to make this legitimately low-power soon.

- also a cc3200 change in here to let C++ constructors work correctly for pinMode, digitalWrite, etc. (not recommended library designers use this, but some Arduino libraries unfortunately do that; Tiva already had the fix in E13)

[igor 163]

@spirilis 

 

I notice that the service pack version changes from reading the start time as early as possible in delayMicroseconds, to reading it a little bit later.

(changes from this version http://forum.43oh.com/topic/6011-tiva-delaymicroseconds-unreliable/#entry52527

to this slightly older version http://forum.43oh.com/topic/6011-tiva-delaymicroseconds-unreliable/#entry52526 )

 

(Difference is whether the 

unsigned long startTime = HWREG(NVIC_ST_CURRENT) ;

appears before checking for long delays, or after.  Of course if it is before, then a re-read of the register is needed if a long delay was used.)

 

If moving that check a little later in the code was intentional - fine, (it makes the code a little more compact.  I don't know how much time difference it makes, suspect it won't make a lot of difference - time for a compare, jump, and probably a pipeline refill).  Just wanted to be sure it wasn't inadvertent (change seems to have come when backported the CC3200 version).

 

Thanks for all the work on this.

 

--

On a totally different topic

 

- also a cc3200 change in here to let C++ constructors work correctly for pinMode, digitalWrite, etc. (not recommended library designers use this, but some Arduino libraries unfortunately do that; Tiva already had the fix in E13)

 

 

 

Is there some place I can read more about this topic (the advisability of pinMode in constructors)?  It sounds like putting pinMode and digitalWrite in a constructor may be frowned upon in library design?  Is it frowned on in Arduino in general, is it just in Energia that it is not recommended?  I was exploring this issue regarding a library I was revising.  The conclusion I came to (based on the arduino documentation) was that pinMode, etc. in a constructor was approved of (maybe even encouraged) in libraries.  This item sounds like perhaps there is more information on this topic which I missed.  If the answer is involved and there isn't already something about it that you have a quick link to, I should probably start a separate thread (or fork this one) to ask the question.  Thanks again.

[spirilis 1,265]

@@igor I moved the start time back after I noticed a very slight *increase* in the delay with the starttime declared at the beginning, which is puzzling.

 

As for constructor init of pinMode et al, this is an Arduino thing in general, and I found discussions on the arduino forum referring to it (would have to search again to find it). In any case, one issue with C++ constructors is you can't predict their order of execution - so it's generally advised to perform only variable initialization within a constructor and nothing that alters hardware state. Complete init is meant for a .begin() function. I've seen some constructors actually perform delay() - what if you had multiple libraries do that? Could have some long startup delays...

 

Sent from my Galaxy Note II with Tapatalk 4

[spirilis 1,265]

I don't think this was the exact thread I had in mind, but here's some examples:

 

http://forum.arduino.cc/index.php?topic=78114.0

 

http://forum.arduino.cc/index.php?topic=142937.0

 

 

 

To elaborate...

The constructor is called before the main() function which does all the configuration and initialisation of the Arduino.

Anything you put in a constructor has a chance (depending on what it is) of being either over-written by defaults set in main(), or of not doing anything at all because it relies on initialisation that hasn't been performed yet.

Anything which is "arduinoesque", i.e., anything like pinMode(), digitalWrite() etc will deffinately not work in a constructor.  The loosely agreed standard is to provide a .begin() method which you call from setup() to configure any IO etc that needs to be configured.  You can either move your setting parameters to that method, or keep them in the constructor but store them in properties for the .begin() method to use later.

[spirilis 1,265]

So in practice, those pinMode's et al often do work inside constructors (with Arduino AVR and MSP430 whose peripheral clocking systems are automatically configured behind the scenes), but as we've seen with Energia support on Tiva and CC3200, that assumption is fundamentally a bad one -- because an "out of the box" C++ application system will crash & burn when such assumptions are used on ARM, unless special measures are taken (in this case moving HW init and GPIO peripheral module clock initialization into a separate _init() function and modifying the Reset ISR to execute _init() before the C++ ctors)