Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Floating point operations in ISR (IDFGH-60) #722

Closed
simon-jouet opened this issue Jun 21, 2017 · 9 comments
Closed

Floating point operations in ISR (IDFGH-60) #722

simon-jouet opened this issue Jun 21, 2017 · 9 comments

Comments

@simon-jouet
Copy link

Hi,

I've been porting some code from an existing arduino codebase to the ESP32 and running into some problems while doing float operations in an ISR. (#562)

This issue isn't particularly recent and looking around it's been mentioned here https://www.esp32.com/viewtopic.php?f=19&t=1292&p=5993&hilit=float+isr#p5993 and here https://www.esp32.com/viewtopic.php?f=13&t=831

In my current port I just changed all the float to double to exclude the FPU, it's far from optimal and requires quite a lot of change to the codebase. In one of the forum thread @Spritetm mentions that some thought on fixing the issue was in progress, just wondering if there has been more progress since then?

In the meantime as a temporary solution is it possible to disable the FPU at compile time? it's far from optimal but it would allow the code to remain the same and once floating point operations are allowed in ISR reverting back to normal would be straightforward.

Cheers

@HubbyGitter
Copy link

Having been responsible for a lot of embedded software in my professional life so far, my two cent's worth in this matter is that at least the combination of "floating point operations in ISRs" and "portability" is a complete no-go, and I am unaware of any colleague who would not agree.

It may be the case that "floating point operations in ISRs on a specific target" could be discussed. But personally, I would nevertheless rather avoid them where possible, because there is usually no need for them:

  • ISRs must be kept as short as possible. Always.
  • If it's a real time system, capture integer / fixed point values and process /scale / adjust them later. You need robust behaviour if there are too many interrupts and the ISRs can only play a minor part there.
  • If it's not a real time system, there's even less need to perform floating point operations in the ISR.
  • If you need minimum latency between ISR occurrence and required output / action (such that there shouldn't be a task in between) simply stick to fixed point arithmetics.

Do you have a use case where you really need (and want) this?

@simon-jouet
Copy link
Author

Hi @HubbyGitter,

Thanks for the feedback, I totally agree with what you are describing and previous discussion in a different issue (linked in the original message) resulted in the same outcome.

The reason for this is that I'm porting the Marlin 3d printer firmware to the ESP32 and the existing code is very much tailored to old 8 bit platforms, there is a current effort to move to some HAL for 32 bits support. In the current implementation the stepper actuation is performed in the ISR (which is neither simple nor short) and in the short term I've been trying to get the ISR working in the ESP. Like I said in the original post I got it working by changing floats to double but it's far from optimal and a slightly less dodgy solution in the short term would be nice.

In the longer term and depending on the time I have available (and obviously the interest) I'm thinking of moving away from the ISR for the stepper and rely on either the RMT or LEDc peripherals and a ring buffer continuously filled by a task, should be much more reliable and lightweight on the CPU.

What I might try to do next is to manually save the registers of the FPU (as shown inhttps://www.esp32.com/viewtopic.php?t=1292) I had a shot at it a month or so ago without much success but I might have another go.

@HubbyGitter
Copy link

@simon-jouet I see... Have you considered putting the ISR code into a high-priority taks (maybe even on a dedicated ESP32 core if that's really necessary) and having the ISR just trigger that task (or maybe collect some initial data including a time stamp if required, and the task could then even wait for "the right time to continue" if required)? That should allow you to stick closely to the original implementation, no?

@simon-jouet
Copy link
Author

@HubbyGitter thanks for the suggestion, I might try it this way, that's close to what @Spritetm was suggesting in the previous issue, however I'm not sure it's worth putting the effort moving to a slightly less broken solution that spending a bit more effort doing it the right way. Anyway thanks very much for your input I will definitely keep in mind when I go back to work on this.

@FayeY FayeY changed the title Floating point operations in ISR [TW#13514] Floating point operations in ISR Jun 26, 2017
@comcomservices
Copy link

@HubbyGitter, I just want to say that I disagree with your point about floating point not being needed in ISR's, I agree that you "can" work around the limitation by using fixed point arthritic and then processing later but in many cases floating point operations in the handler can clean up code significantly.

We use the ESP32 in energy monitoring applications where high speed analog acquisition is used and vectors such as instantaneous power need to be calculated on a sample by sample basis. These calculations cannot be made without scaling the analog inputs to there proper units, Amps, Volts etc.. You could do weird workarounds by making one unit x times larger then the other to keep a fixed ratio and then divide later but it just makes everything so messy, especially as you get into THD analysis, realtime voltage protection etc.

Using mutexes etc to process data once the read is finished is simply impossible as the service latency is way to high..

On previous platforms we had been using, Cortex m4, m7 performing FP and DFP operatins in interrupt handlers was common practice for these types of applications..

We use the following code to work with floating point vectors in an ISR..

`
uint32_t cp0_regs[18];

portENTER_CRITICAL_ISR(&afeCriticalMutex);
uint32_t cp_state = xthal_get_cpenable();

        if(!cp_state) {
            xthal_set_cpenable(1);
        }

        xthal_save_cp0(cp0_regs);   // Save FPU registers

// Work with FPU

        xthal_restore_cp0(cp0_regs);

        if(cp_state) {
            // Restore FPU registers

        } else {
            // turn it back off
            xthal_set_cpenable(0);
        }
portEXIT_CRITICAL_ISR(&afeCriticalMutex);

`

@projectgus projectgus changed the title [TW#13514] Floating point operations in ISR Floating point operations in ISR (IDFGH-60) Mar 12, 2019
@filzek
Copy link

filzek commented Aug 4, 2019

@comcomservices did tou use FP on ESP32 inside hw_timers ISR without any issues? Are they precise?

@comcomservices
Copy link

comcomservices commented Aug 4, 2019 via email

@filzek
Copy link

filzek commented Aug 4, 2019

@comcomservices it seens FPU are not quite precise as software double is, did you try to use latest DEV 4.0 to see if the problem was solved?

@igrr
Copy link
Member

igrr commented Sep 22, 2020

Implemented in 86034ad as a configurable option (disabled by default).

@igrr igrr closed this as completed Sep 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants