Re: [PATCH 10/10] samples: rust: platform: Add property read examples

From: Rob Herring
Date: Wed Mar 26 2025 - 20:03:22 EST


On Wed, Mar 26, 2025 at 5:24 PM Remo Senekowitsch <remo@xxxxxxxxxxx> wrote:
>
> On Wed Mar 26, 2025 at 11:01 PM CET, Rob Herring wrote:
> >>
> >> + let prop = dev
> >> + .property_read::<bool>(c_str!("test,bool-prop"))
> >> + .required()?;
> >
> > The 'required' is kind of odd for boolean properties. They are never
> > required as not present is the only way to to get false.
>
> Agreed. I can think of a few alternatives:
>
> * Make the trait `Property` more flexible to allow each implementor to specify
> what its output type for the `read` function is, via an associated type.
> I really don't like this idea, because overly generic APIs can mess with type
> inference and become less ergonomic because of it.
>
> * Use `propert_present` instead. That doesn't perfectly express the intention,
> because it doesn't warn if the property is present but has a type other than
> bool.

Right. I've been cleaning up the tree to use of_property_read_bool()
on bools and of_property_present() on non-bools, so don't want to go
back to 1 function. The C code now warns on a mismatch.

> * Add an additional inherent method `property_read_bool`, which returns a plain
> `bool` instead of `PropertyGuard<bool>`. Then there will be three slightly
> different ways to read a bool: `property_present`, `property_read_bool` and
> `property_read::<bool>`. Maybe that's confusing.
>
> * Add `property_read_bool` and remove `impl Property for bool`. That would avoid
> confusion between `property_read_bool` and `property_read::<bool>`, only the
> former would work.

I think I would go with this option. Easier to add another way later
than remove one.

Rob