V3.0.0 alpha2 Issues with esp32-hal-timer "update"

[dagnall53]

Board

ESP32 devboard

Device Description

Board is a data multiplexer

Hardware Configuration

N/A

Version

latest master (checkout manually)

IDE Name

2.2.1

Operating System

win10

Flash frequency

N/A

PSRAM enabled

yes

Upload speed

115200

Description

esp32-hal-timer library seems to have been significantly changed, which affects how previously good code will now compile.
I am not sure why this has occurred, but my code exposed a number of functions whose definitions have been (?unnecessarily?) altered in V3.0.0.
I would request that the "updates" for esp32-hal-timer be reviewed in detail for consistency with the previous versions, and corrected to ensure backwards compatibility and restore functionalty!
The specific functions that I noted had changed significantly were:
timerBegin
Changed from timerBegin(uint8_t num, uint16_t divider, bool countUp){
to timerBegin(uint32_t frequency){ !!
timerAlarmWrite Completely missing in new library
timerAlarmEnable Completely missing in new library

Comparison of the old esp32-hal-timer from 2.0.14 to the new one shows many other (? unnecessary ?) changes.

First noted in #8765

Sketch

example in hardware serial detector:

void IRAM_ATTR onRx() { 
  detachInterrupt(rxPin); 
  rxBitCounter = 1;  
  rxByte = 0;  
  timerStart(timer_1);
  timerAlarmWrite(timer_1, ini_time, true);  
  timerAlarmEnable(timer_1);
  last_ST_time = micros();  
}

Debug Message

N/A see https://github.com/espressif/arduino-esp32/issues/8765

Other Steps to Reproduce

No response

I have checked existing issues, online documentation and the Troubleshooting Guide

• I confirm I have checked existing issues, online documentation and Troubleshooting guide.

[me-no-dev]

Changes were required because of changes also in ESP-IDF. The new API is more Arduino-esque and conscious. Unfortunately sometimes breaking changes are necessary and here we are coming with a new major version, so it is allowed. Backwards compatibility is not always possible.

@P-R-O-C-H-Y please help to convert to 3.0.0 code.

[P-R-O-C-H-Y]

@dagnall53 I will just change your code snippet to this:

void IRAM_ATTR onRx() {
  detachInterrupt(rxPin);
  rxBitCounter = 1; 
  rxByte = 0; 
  timerStart(timer_1);
  timerAlarm(timer_1, ini_time, true, 0);  //Alarm is automatically enabled and the last input 0 means unlimited reload count
  last_ST_time = micros();
}

For more information about new API you can check the Timer documentation

[dagnall53]

@P-R-O-C-H-Y (thanks for code suggestion I will record that suggestion in my code in case I need to use it. )
@me-no-dev (thanks for reading! I hope you will continue to educate me!)

(This is not a rant, just an honest set of questions!)

First, and least problematic - timerAlarm() - This revised function will add a new "reload count limit" function that will be useful for some people, and retain the "number to count to". However, I would hope that the final esp32-hal-timer code could also retain the preexisting timerAlarmWrite() definition, to allow unmodified reuse of old source: such as the software serial port libraries.
Yes, I could define it (and other missing functions) in my own source, but why does the new API have to delete a lot of pre-existing functions just to use the new V3.00 compiler, when it could (?apparently easily?) define them again in the master API as before? e.g for this case - unless I have misunderstood.. --
void timerAlarmWrite(hw_timer_t *timer, uint64_t alarm_value, bool autoreload){
timerAlarm(timer,alarm,autoreload, 0); // use new timerAlarm function with reload count infinite.
}

timerBegin() Here I am very confused. -
OLD API: timerBegin set (which HW timer to use, divider and direction)
NEW API timerBegin sets (Frequency). (only?? - no selection of which HW timer - how to use more than one??
I do not do code professionally so I must assume I have missed some important issues / points. but I can see (I think) that that whole API is now based around the premise that the timers are configured with gptimer_config_t - a VERY different structure to the old timer_config_t, and seemingly replacing a simple divider/count direction etc setting with a Frequency based setting! This appears to be a very different approach to the old API and one that appears to add significant (unnecessary?) complication for the API !
??WHY?? what value does this change add? it apparently makes all legacy timer code redundant for no apparent reason. - I can only assume I have significantly misunderstood some key point ! -- .

Thanks for the discussion and education!
Dagnall

[me-no-dev]

here on your confusion:

ESP-IDF used to also access the timers by number, so it made sense to keep that the same. Now though, ESP-IDF allocated those resources and gives you the next free timer instead. This makes sense for many reasons, but also required us to change API. So now you just ask for new timer (with timerBegin) and do not care which number it is.

[me-no-dev]

On the first:

Since we already had to change API of some of the timer functions (read above) it made sense to also rename some of them to make more sense and follow better the Arduino style. Libraries that use the timers will need to adjust their code to support the new API now anyway, so doing that for one more function is not that problematic. Plus they might figure out that the new options fit their needs better.

[P-R-O-C-H-Y]

Also 2 more benefits of timer in 3.0:

• now all timer functions are allowed to run within ISR context (in 2.X some timer functions were forbidden to run within ISR context).
• as now input is the frequency, we are selecting the clock source for the timer (to match the desired frequency) to get greater frequency range than it was in 2.X.

[dagnall53]

@P-R-O-C-H-Y @me-no-dev
Many thanks.. I guess I will have to wait for V3, but your notes help me understand the “why” for these changes. Not having to specify “which” timer is very sensible, and I think I have already had issues in the past with timers and ISR! - Im not sure how many Software Serial users and libraries there are, but I presume they will all need updating! We will all be converting from our Baud to “count and divider data” to just 1/baud.. so that will be nice!. But I hope we will not run into any integer division issues and can keep our accuracies!

[TD-er]

Another great benefit is that every access to such timers should now use the same code to see which timer is free.
So you don't have to and also should not experience some library suddenly taking an occupied timer.

[lucasssvaz]

@dagnall53 The migration guide to 3.0.0 is now available here

[dagnall53]

@lucasssvaz thanks. Can I check your development terminology please. Do you use the term 'Breaking Changes' to mean changes that BREAK previous code?
Or the more normal use of 'these are changes that are approaching' ?
Just wondering? !

[me-no-dev]

Breaking Changes when we talk about software versioning means that the old code will no longer compile or will not work properly.

[dagnall53]

Thanks !
Much appreciated!

[VojtechBartoska]

I think we can close this issue as answered. :)

[dagnall53]

@VojtechBartoska @me-no-dev
Can I just ask that in addition to the highly simplified Timer example codes, in the guide you also add a SOFTWARE SERIAL port example.
I think attempting to do this with the new API would be helpful and instructive.
I have spent a lot of time getting SW serial ports to work through all the little "quirks" in previous compilers and (like many I suspect!) am not prepared to go to V3 until key elements of the new API concepts have been demonstrated to be reliable and robust.
Adding Breaking Changes like this is damaging the reputation of Expressive. I have seen comments on many developer websites comments about ESP32 code now being broken and how they now plan to avoid this (compiler update - and implied use of ESP32 for future?) for future devices.. This would be unnecessary if you can retain a legacy code backwards compatibility mode.
(rant over!)

[me-no-dev]

@dagnall53
If we are to say that we provide such example, then we are responsible for that example to behave properly in various conditions and for a whole protocol like UART, it goes a bit beyond the scope of the core examples and is well handled externally. I hope this makes sense :)

[dagnall53]

@me-no-dev Thank you for replying!
I do understand, but the changes to the timers will catastrophically break all existing hardware timer codes.
Software uarts are just one example. By providing a “worked example” using the new functions would make it more probable that people will progress to V3. Otherwise they will all stay at the last V2. Which will completely waste all your development efforts.
An even better solution would be to provide some backwards compatible functions.

[me-no-dev]

Do you see anything that you can do with the old API and can not do with the new one? It would make more sense to show that, than writing a whole soft serial. API has changed, but it's still a hardware timer that does the same thing

[dagnall53]

Good question.
But it is more the fact that libraries that use the old methods will now fail.
This is sort of an unknown unknown problem! I do not know what everyone uses! I can probably change my own code, but my co-programmer is already reluctant to move from 2.0.4. For fear of introducing problems.

[me-no-dev]

But it is more the fact that libraries that use the old methods will now fail.

and that is OK, because we are incrementing a major version that permits such breaking changes. The fact that we have changed some APIs in no way means that you will not be able to have SoftwareSerial or anything else. We provide header to check against the core version and libs can accommodate old and new support at the same time. We could not stay with the old API and progress is a normal thing.