jonas-schievink / rubble

(going to be a) BLE stack for embedded Rust
BSD Zero Clause License
397 stars 56 forks source link

Should we make the Attribute value generic? #153

Open daschl opened 3 years ago

daschl commented 3 years ago

Hi,

After working with Attribute for a bit, especially when dynamically modifying the value, I find it very challenging to use because it takes a 'a [u8], so it pushes the burden upwards on where to store the slice? In my experience this always ends up being somewhere static, making it a bit hard to use with RTIC resources (or maybe my rust foo isn't good enough).

So as an example I want to read my boiler temp and then set it on the attr:

    #[task(resources = [ble_r, boiler_sensor, temp_timer, boiler_temp], priority = 2, schedule = [measure_boiler_temp])]
    fn measure_boiler_temp(ctx: measure_boiler_temp::Context<'static>) {
        if let Ok(t) = ctx.resources.boiler_sensor.read(ctx.resources.temp_timer) {
            let rounded_t = unsafe { t.as_celsius().to_int_unchecked::<u16>() };
            defmt::info!("Temp is: {:u16}", rounded_t);

            *ctx.resources.boiler_temp = rounded_t.to_le_bytes();

            let l2cap = &mut *(ctx.resources.ble_r.l2cap());
            let provider: &mut ControllerServiceAttrs<'static> = l2cap.channel_mapper().attribute_provider();
            provider.set_boiler_temp((*ctx.resources.boiler_temp).as_ref());
        }

        ctx.schedule
            .measure_boiler_temp(ctx.scheduled + TEMP_MEASURE_PERIOD.cycles())
            .unwrap();
    }

I'm having issues with static being required:

180 |             provider.set_boiler_temp((*ctx.resources.boiler_temp).as_ref());
    |                      ^^^^^^^^^^^^^^^ lifetime `'static` required

So I'm wondering if we can/should make it generic so it would be possible to store i.e. an array of two bytes in it instead of the slice, as long as the type can implement representing its value as a slice?

jonas-schievink commented 3 years ago

Yes, I only made this a byte slice because that is always the underlying encoded data. The whole API needs an overhaul.