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

Interrupts broken on latest Arduino Libraries #11

Closed
davidgs opened this issue Jul 23, 2019 · 11 comments
Closed

Interrupts broken on latest Arduino Libraries #11

davidgs opened this issue Jul 23, 2019 · 11 comments
Assignees
Labels

Comments

@davidgs
Copy link

davidgs commented Jul 23, 2019

On the latest Arduino Libraries, using interrupts has changed significantly. Your code generates : `ISR not in IRAM!

Abort called`

I'm looking at how to fix it, but so far have not found a solution.

@MonsieurV
Copy link
Owner

MonsieurV commented Jul 23, 2019

Hi @davidgs

It's been awhile I've not played with my Arduino(s).

Does it happens at compile time? Or while uploading or running the program?
What Arduino do you use?

@MonsieurV MonsieurV added the bug label Jul 23, 2019
@MonsieurV MonsieurV self-assigned this Jul 23, 2019
@davidgs
Copy link
Author

davidgs commented Jul 23, 2019

It happens at runtime because the interrupt handlers now need to be defined:
ICACHE_RAM_ATTR void onNoise() { Serial.println("Argh, noise, please stop moving"); }

The library doesn't handle this new RAM_ATTR setting for callback handlers. This will also impact how values are handled since you can't really Serial.print() from within these handlers anymore (they are true interrupts now).

@MonsieurV
Copy link
Owner

MonsieurV commented Jul 24, 2019

The external handlers (registered with registerRadiationCallback and/or registerNoiseCallback) are actually called from within the library loop: https://github.com/MonsieurV/ArduinoPocketGeiger/blob/master/src/RadiationWatch.cpp#L92
They are not true interrupts. So there shouldn't be an issue here.

However indeed the _internal_handlers are true interrupts and are not defined with ICACHE_RAM_ATTR for the moment: https://github.com/MonsieurV/ArduinoPocketGeiger/blob/master/src/RadiationWatch.cpp#L21

I try a fix and I'll invite you to test it.

PS. Can you tell me which board do you use to test? This seems a esp8266 related only issue. source1 source2 source3 (to install the board: instructions)

@MonsieurV
Copy link
Owner

@davidgs could you test the library with the patch from the branch https://github.com/MonsieurV/ArduinoPocketGeiger/tree/ESP8266_irq?

(To test with this library, download or clone the repository from the branch ESP8266_irq to your Arduino library folder)

I've added the ICACHE_RAM_ATTR flag to interrupt handlers (only on ESP8266 boards): https://github.com/MonsieurV/ArduinoPocketGeiger/pull/12/files#diff-f7bdcb1c8355d9520cc88cd8539d7dacL20

@davidgs
Copy link
Author

davidgs commented Jul 24, 2019

I'm running it now, and it throws no errors (progress!) but it also registers nothing. I can even tap on the sensor and it generates no noise interrupts.

@MonsieurV
Copy link
Owner

Hum, interesting. I wonder if the interrupts are well defined and/or registered.

If you add a debug Serial.println("Declaring IRQ handlers"); in between #if defined(ESP8266) and #else, do you see the statement executed?

Can you have a way to validate the wiring by creating a simple sketch program just to see if IRQ are working? (For eg. setting an IRQ on the noise pin, then incrementing a counter or setting a flag and printing it from the main loop, just to see if the IRQ is triggered)
I can try to provide a snippet

@davidgs
Copy link
Author

davidgs commented Jul 24, 2019

I'm running this code, and interestingly, if I tab on the sensor, I get the series of Declaring IRQ Handlers messages. Every Gamma Ray count also prints the Declaring IRQ Handlers message.

/*
This simple example tell you on serial each time a gamma radiation hits the
Pocket Geiger and print the current micro Sievert dose. It also notify you of
the presence of vibration that prevent proper measurements.
*/

RadiationWatch radiationWatch(D1, D2);

void onRadiation()
{
  Serial.println("A wild gamma ray appeared");
  Serial.print(radiationWatch.uSvh());
  Serial.print(" uSv/h +/- ");
  Serial.println(radiationWatch.uSvhError());
}

void onNoise()
{
  Serial.println("Argh, noise, please stop moving");
}

void setup()
{
  Serial.begin(115200);
  Serial.println("Starting Radiation Watch ...");
  radiationWatch.setup();
  // Register the callbacks.
  radiationWatch.registerRadiationCallback(&onRadiation);
  radiationWatch.registerNoiseCallback(&onNoise);
}

void loop()
{
  radiationWatch.loop();
}`

The output is: 

Starting Radiation Watch ...
Declaring IRQ handlers
Declaring IRQ handlers
Declaring IRQ handlers
Argh, noise, please stop moving
A wild gamma ray appeared
0.00 uSv/h +/- 0.00
Declaring IRQ handlers
A wild gamma ray appeared
0.05 uSv/h +/- 0.05
Declaring IRQ handlers
Declaring IRQ handlers
A wild gamma ray appeared
0.08 uSv/h +/- 0.05
Declaring IRQ handlers
A wild gamma ray appeared
0.10 uSv/h +/- 0.05
Declaring IRQ handlers
A wild gamma ray appeared
0.12 uSv/h +/- 0.05
Declaring IRQ handlers
A wild gamma ray appeared
0.13 uSv/h +/- 0.05
Declaring IRQ handlers
Declaring IRQ handlers
A wild gamma ray appeared
0.11 uSv/h +/- 0.04
Declaring IRQ handlers
A wild gamma ray appeared
0.12 uSv/h +/- 0.04
Declaring IRQ handlers
A wild gamma ray appeared
0.10 uSv/h +/- 0.03


> 

@MonsieurV
Copy link
Owner

Oh, very interesting! Seems like it's now doing something.

And if you remove the debug statement, does it works now?
Because there are clearly interrupts working in your log. (and the conditional macro seems to work)

@davidgs
Copy link
Author

davidgs commented Jul 25, 2019

Yes, without the debug it seems to work just fine. It's been running now for 24 hours without incident. I believe the same interrupt situation is going to present itself on ESP32 systems. I'm about to move this over to an ESP 32 so I'll let you know. A conditional Macro for ESP32 might also be required. I'll let you know what I find.

@MonsieurV
Copy link
Owner

Ok, so our fix seems to work. We can easily roll out to other platforms if needed.

I'll wait for your test on ESP32 before merging and releasing a new version :)

MonsieurV added a commit that referenced this issue Jul 31, 2019
Move interrupt handlers to RAM for ESP8266 platform #11
@MonsieurV
Copy link
Owner

I've released a new version with the patch.
https://github.com/MonsieurV/ArduinoPocketGeiger/releases/tag/v0.6.2
It should be available soon from Arduino libraries: https://www.arduinolibraries.info/libraries/radiation-watch

Thanks for the report @davidgs! 👍

Feel free to re-open / open a new issue for ESP32.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants