olliw42 / mLRS

2.4 GHz & 915/868 MHz & 433 MHz/70 cm LoRa based telemetry and radio link for remote controlled vehicles
GNU General Public License v3.0
279 stars 58 forks source link

Esp interrupt cleanup #152

Closed jlpoltrack closed 3 months ago

jlpoltrack commented 3 months ago

Rationale: ESP32 doesn't like noInterrupts / Interrupts functions.

This removes them from the ESP8266 code, tested with Generic 900.

If this makes sense, will also need to update all of the Hals to detach the DIO interrupt.

olliw42 commented 3 months ago

I think we had this discussion, at least it looks to me we did ... and it totally does not make sense to me. Not protecting a critical block is just not ok. Frankly, to me it looks like we just don't know how to do it correctly. To me it looks like we play always just wth the same functions, but that the important piece of how to do it is not yet discovered.

olliw42 commented 3 months ago

also I think this shouldn't go into main but a esp dev branch. IMHO we shouldn't fumble with main until we see how it all unfolds and pieces together for esp32

jlpoltrack commented 3 months ago

also I think this shouldn't go into main but a esp dev branch. IMHO we shouldn't fumble with main until we see how it all unfolds and pieces together for esp32

Good point, I will close this PR.

I understand your point on the critical RXClock piece. Are you strongly against the DeInit concept?

olliw42 commented 3 months ago

Are you strongly against the DeInit concept?

not neccessarily, I just don't see it should be needed

I guess I will do a search in the elrs code base for attachInterrupt and deattach, I feel we are missing something