Register Write Limitation

[AdinAck]

Moved from stm32-rs-nightlies

I ran into an issue when writing to a register where I wanted to return a value from the write, like this:

let val = reg.write(|w| {
    T::set(w.field())
});

But, the signature of write forbids this:

pub fn write<F, T>(&self, f: F)
    where
        F: FnOnce(&mut REG::Writer) -> &mut W<REG>,

So I'm curious, why not change the signature to:

pub fn write<F, T>(&self, f: F) -> T
    where
        F: FnOnce(&mut REG::Writer) -> T

By implementing it like this:

pub fn write<F, T>(&self, f: F) -> T
where
    F: FnOnce(&mut W<REG>) -> T,
{
    let mut writer = W {
        bits: REG::RESET_VALUE & !REG::ONE_TO_MODIFY_FIELDS_BITMAP
            | REG::ZERO_TO_MODIFY_FIELDS_BITMAP,
        _reg: marker::PhantomData,
    };
    let result = f(&mut writer);

    self.register.set(writer.bits);

    result
}

Is there a drawback to this?

[burrbull]

https://github.com/rust-embedded/svd2rust/pull/738#issuecomment-1624819353

[AdinAck]

I don't believe my proposal is related to that. I don't want to return the writer, in fact, the proposed body of write forbids T ever being W or &mut W since bits is moved out of the writer for register.set, no?

[AdinAck]

Ok yes I believe I set up an analogous setup in rust playground:

#[derive(Debug)]
struct Bits(u32);

#[derive(Debug)]
struct W {
    bits: Bits,
    _reg: (),
}

fn consume_bits(Bits(num): Bits) {
    println!("bits: {}", num);
}

fn write<F, T>(f: F) -> T
where
    F: FnOnce(&mut W) -> T,
{
    let mut writer = W { bits: Bits(0), _reg: () };
    
    let result = f(&mut writer);
    
    consume_bits(writer.bits);
    
    result
}

fn main() {
    let outside = write(|_w| 0xaa);
    // let outside = write(|w| w); <- does not compile
    println!("outisde: {:?}", outside);
}

So I don't believe this introduces any complications as mentioned in #738.

[burrbull]

Sorry for waiting. Looks interesting. I think you could nominate it for next meeting: https://github.com/rust-embedded/wg/discussions/800

[AdinAck]

No problem, you have a lot oh your hands. I've never done that before so I don't know the proper procedure.

[burrbull]

Do your suggestion require changing of signatures of write/modify only or field writers too?

In other words can we use traditional field modifying system simultaneously with yours?

[AdinAck]

Just write and modify, all the field writers work as is!

[AdinAck]

I'm going to turn this issue into a PR so exactly what I'm proposing is visible.

[burrbull]

I understand what you want, but I not sure what is better:

• replace existent write and modify or
• add additional write and modify with new signature and other names

I prefer second as less breaking.

[AdinAck]

Ah I hadn't even thought of that, two good options!

I agree, it took me like 3 hours to convert all write and modify call sites in the stm32g4xx-hal, wouldn't want others to go through that.

I'll still make a PR but as an addition rather than a change.

[AdinAck]

Resolved by #874.

[burrbull]

@AdinAck I've released current state of stm32-rs G4 as stm32g4-staging

You could use it as:

[dependencies.stm32g4]
package = "stm32g4-staging"
version = "0.17.0"