diondokter / device-driver

A toolkit to create better Rust device drivers, faster
https://diondokter.github.io/device-driver/
Apache License 2.0
132 stars 5 forks source link

Aliasing RO/WO bits cause codegen failure with `generate(Debug)` #13

Closed korken89 closed 10 months ago

korken89 commented 3 years ago

I have noticed that generating dual definitions within a register works fine, for example:

        // #[generate(Debug)]
        tuning_r(RW, 0x1d, 1) = {
            dl_tune: u8 = RW 4..=7,
            lead_lag: u8 = RO 3..=3,
            cagainov: u8 = WO 3..=3,
            integlen: u8 = RW 2..=2,
            integgain: u8 = RW 0..=1,
        },

The bit which has different meaning when writing and reading is generated correctly in the R and W. However when I add the #[generate(Debug)] the debug tuple is not correctly constructed with the debug info for the WO bit is in the R debug print impl:

            impl core::fmt::Debug for R {
                fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> Result<(), core::fmt::Error> {
                    f.debug_struct("tuning_r::R")
                        .field(
                            "raw",
                            &device_driver::utils::SliceHexFormatter::new(&self.0),
                        )
                        .field("dl_tune", &self.dl_tune())
                        .field("lead_lag", &self.lead_lag())
                        .field("cagainov", &self.cagainov())
                        .field("integlen", &self.integlen())
                        .field("integgain", &self.integgain())
                        .finish()
                }
            }

So to me it seems that the #[generate(Debug)] does not handle WO bits correctly as they should not be in the debug print. What do you think @diondokter ?

diondokter commented 3 years ago

Ah right! It's pretty obvious spelled out like this. I did not think about checking the field RW state so it currently tries to show all fields regardless.

Gonna need an extra macro to fix 😅 But this is also doable. Will look into it.

Thanks for spotting it!

diondokter commented 3 years ago

So, I had some time (and motivation) today. Sadly the way I thought I was gonna solve this doesn't work. I may have to do a really ugly workaround. But that's ok if I'm gonna port this to a proc macro after this...

Sorry for taking so long :(

korken89 commented 3 years ago

No problem!

diondokter commented 10 months ago

Crate has been overhauled. Issue no longer relevant