Compile errors platforms without `critical-section` support (was: Support Arduino?)

Laura7089 commented 2 years ago

I'm using arduino-hal and shared-bus to manage two devices on the same i2c bus. Is this supported? Am I doing something wrong? When compiling I get:

   Compiling critical-section v0.2.5
error: Critical section is not implemented for this target. Make sure you've specified the correct --target. You may need to supply a custom critical section implementation with the `custom-impl` feature
   --> /home/laura/.cargo/registry/src/github.com-1ecc6299db9ec823/critical-section-0.2.5/src/lib.rs:149:9
    |
149 | ...   compile_error!("Critical section is not implemented for this target. Make sure you've specified the correct --target. You may need to supply a custom critical section implementation with the `custom-impl` feature");
    |       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

error: could not compile `critical-section`

Laura7089 commented 2 years ago

Note: I'm compiling against the avr-atmega328p target for the nano, and using nightly 2020-12-22 as in the avr-hal template repo.

Rahix commented 2 years ago

Hi,

before diving into the actual issue here, just a quick question:

and using nightly 2020-12-22 as in the avr-hal template repo.

I am curious what template you're referring to here? The template I know of is https://github.com/Rahix/avr-hal-template which specifies nightly 2021-01-07...

Okay, now regarding the real problem: The compiler error you are encountering is an unfortunate side-effect of the usage of the critical-section crate since the last release... For your use-case, you most likely do not care about any of the code using this dependency (just relevant for RTIC use). So if it just wasn't there or was feature-gated, you could at least use the BusManagerSimple without any problems.

There are three solutions to this:

Gate the relevant code for architectures which are not yet supported by critical-section. This would at least restore functionality of the rest and would not be a breaking change in my eyes.
Add support for AVR in critical-section. This is of course the long-term plan but the problem would remain for all other unsupported architectures... I am also not a fan of adding an experimental architecture to critical-section just yet when it does not even compile with current compilers...
Add a feature-flag for the offending AtomicCheckMutex so it becomes opt-in. This would probably be the cleanest change but it would entail a breaking change and thus a big version bump.

I am personally favoring version 3 as it will prevent any headaches with other architectures in the future...

That said, there is a 4th solution just for you if you need it working quickly: If you pin your dependency to an older version with

[dependencies]
shared-bus = "=0.2.2"

it should hopefully also just work for the time being...

Laura7089 commented 2 years ago

Wow, thanks for the detailed response! The rust version mismatch is my mistake.

I've played about with critical-selection's custom-impl, and this project. I couldn't get the necessary AVR ASM to compile to set the status register flag we'd need to turn off interrupts.

I agree with the preference for 1 or 3 - as regards 2, what is the current stability of AVR in rustc and where can I track that?

Rahix commented 2 years ago

I couldn't get the necessary AVR ASM to compile to set the status register flag we'd need to turn off interrupts.

If you are still curious, you can take a look at the relevant code in avr-device: https://github.com/Rahix/avr-device/blob/main/src/interrupt.rs

what is the current stability of AVR in rustc and where can I track that?

Hard to tell for me as well... The O-AVR label in the rust repo maybe gives a bit of an indication of where the problems are. A lot has happened in LLVM upstream already but has not yet trickled down into rustc.

The biggest issue is probably https://github.com/rust-lang/compiler-builtins/issues/400 which is still being worked on in LLVM: https://reviews.llvm.org/D114611 Everyone is waiting on that to land as this is what is preventing newer nightly compilers from working...

With the nightly-2021-01-07 compiler that most people are using right now, you can roughly expect the following:

Do not use f32 or f64 as they do not behave properly in all cases. The Fixed<i16> type from the fixed crate is a very good alternative for most users.
Large integer types (>16 bit) also sometimes have problems although this was improved a lot with a workaround from https://github.com/rust-lang/rust/issues/82242#issuecomment-997400373 (that should be present in the avr-hal template already).
Trait objects and function pointers do not work. Generics are okay but don't use dyn.
Interrupt Service Routines have problems with overwriting registers which they shouldn't.

There might be more which I have forgotten about...

Bottom line is this, I guess: If you need things to work, are on a schedule, and/or your application is rather complex, AVR microcontrollers are not the best choice with Rust right now. For just playing around with an Arduino a bit it is fine, but I wouldn't attempt any large and complex projects with it yet...

Laura7089 commented 2 years ago

Again, thanks for the detailed reply. It's a shame about the compatibility issues; I guess I'll just have to keep an eye on it.

Rahix commented 2 years ago

With release 0.2.4 this should at least be fixed to the point where shared-bus can be used in single-threaded mode on AVR again.

Rahix / shared-bus

Compile errors platforms without `critical-section` support (was: Support Arduino?) #31