usbd-human-interface-device icon indicating copy to clipboard operation
usbd-human-interface-device copied to clipboard

Feature to disable uses of atomic64?

Open hcabel opened this issue 1 year ago • 7 comments

Hey, I would love to use your crate sadly my micro controller esp32-s3 doesn't have support for 64bit atomic. I've tracked it down to the packed_struct crate that uses bitvec which uses those 64bit atomic I dont know how feasible it is but I would love a feature that we can turn on to use 32bit atomic instead or more likely that doesn't use this packed_struct crate :) I understand my chance are slim ^^

hcabel avatar Feb 01 '25 15:02 hcabel

Hi. I've reproduced the compiler errors due to the interaction of the esp32-s3 toolchain and packed_struct/bitvec/radium.

error[E0432]: unresolved imports `core::sync::atomic::AtomicI64`, `core::sync::atomic::AtomicU64`
  --> C:\Users\dlkj\.cargo\registry\src\index.crates.io-6f17d22bba15001f\radium-0.7.0\src\lib.rs:53:34
   |
53 |         use core::sync::atomic::{AtomicI64, AtomicU64};
   |                                  ^^^^^^^^^  ^^^^^^^^^ no `AtomicU64` in `sync::atomic`
   |                                  |
   |                                  no `AtomicI64` in `sync::atomic`
   |
help: a similar name exists in the module
   |
53 |         use core::sync::atomic::{AtomicI32, AtomicU64};
   |                                  ~~~~~~~~~
help: a similar name exists in the module
   |
53 |         use core::sync::atomic::{AtomicI64, AtomicU32};
   |                                             ~~~~~~~~~

error[E0412]: cannot find type `AtomicI64` in module `core::sync::atomic`
    --> C:\Users\dlkj\.cargo\registry\src\index.crates.io-6f17d22bba15001f\radium-0.7.0\src\types.rs:52:41
     |
52   |       if atomic(64) { core::sync::atomic::AtomicI64 }
     |                                           ^^^^^^^^^ help: a struct with a similar name exists: `AtomicI16`
     |
    ::: C:\Users\dlkj\.rustup\toolchains\esp\lib\rustlib\src\rust\library\core\src\sync\atomic.rs:3086:1
     |
3086 | / atomic_int! {
3087 | |     cfg(target_has_atomic = "16"),
3088 | |     cfg(target_has_atomic_equal_alignment = "16"),
3089 | |     stable(feature = "integer_atomics_stable", since = "1.34.0"),
...    |
3102 | |     i16 AtomicI16
3103 | | }
     | |_- similarly named struct `AtomicI16` defined here

error[E0412]: cannot find type `AtomicU64` in module `core::sync::atomic`
    --> C:\Users\dlkj\.cargo\registry\src\index.crates.io-6f17d22bba15001f\radium-0.7.0\src\types.rs:58:41
     |
58   |       if atomic(64) { core::sync::atomic::AtomicU64 }
     |                                           ^^^^^^^^^ help: a struct with a similar name exists: `AtomicU16`
     |
    ::: C:\Users\dlkj\.rustup\toolchains\esp\lib\rustlib\src\rust\library\core\src\sync\atomic.rs:3105:1
     |
3105 | / atomic_int! {
3106 | |     cfg(target_has_atomic = "16"),
3107 | |     cfg(target_has_atomic_equal_alignment = "16"),
3108 | |     stable(feature = "integer_atomics_stable", since = "1.34.0"),
...    |
3121 | |     u16 AtomicU16
3122 | | }
     | |_- similarly named struct `AtomicU16` defined here

I'd be open to moving away from packed_struct. It's only used for 2 non-device structs (HidDescriptorBody & UsbRequest). If you want to take a swing at a PR, I'd be happy to review.

I'd be open to either hand crafting the packing/unpacking code or looking at a library with fewer/lighter weight dependencies. I'd very much like to avoid any unsafe code in the core library, so I expect some 3rd party abstraction would be helpful to keep the implementation as efficient as possible.

If you don't feel up to a PR, I'm happy to keep this open and someone else or I might pick this up at a later date.

dlkj avatar Feb 01 '25 16:02 dlkj

I dont know how feasible it is but I would love a feature that we can turn on to use 32bit atomic instead

From a quick dig though radium, this isn't possible without changes to multiple 3rd party crates.

dlkj avatar Feb 01 '25 16:02 dlkj

Thanks for responding quickly (really appreciate)

Look like your using packed_struct for more than HidDescriptorBody & UsbRequest (see here)

Doing more investigation, I reckon it might more beneficiary to change how packed_struct behave instead of your crate. (also because they use AtomicU64 in only 2 function) I've opened an issue for them, maybe it would be worst doing the same for bitvec? I'm waiting to hear from the packed_struct crate first

In the meantime I've commented the 2 function that block me and everything work fine :) https://github.com/hashmismatch/packed_struct.rs/issues/114

hcabel avatar Feb 02 '25 02:02 hcabel

Having the exact same problem, did you find a work around?

flyaruu avatar Feb 19 '25 09:02 flyaruu

@flyaruu Yep, I downloaded both crate locally -> link them together -> got rid of the bitvec dependency -> and added a panic in both places that where using bitvec (in packed_struct.rs/packed_struct/src/types_num.rs line 667 and 687) Those panic never got triggered once for my limited use

hcabel avatar Feb 19 '25 12:02 hcabel

Thanks!

flyaruu avatar Feb 20 '25 07:02 flyaruu

For whom may encounter this issue in future: I have encountered the same problem and made a fork of packed_struct with changes mentioned by @hcabel. It's three commits older than the upstream since upstream has updated to v0.11 while this crate still uses v0.10

To use the forked version, just slap this snippet to your Cargo.toml:

[patch.crates-io]
packed_struct = { version = "0.10.1", git = "https://github.com/George-Miao/packed_struct.rs.git" }

And it should work again.

George-Miao avatar Sep 28 '25 19:09 George-Miao