Re: [PATCH v7 5/6] iio: ABI: Add partial quaternion modifier

[David Lechner]

On 3/4/26 2:07 AM, Francesco Lavra wrote:
> This modifier applies to the IIO_ROT channel type, and indicates a data
> representation that specifies the {x, y, z} components of the normalized
> quaternion vector.
> 
> Signed-off-by: Francesco Lavra <flavra@xxxxxxxxxxxx>
> ---
>  Documentation/ABI/testing/sysfs-bus-iio | 13 +++++++++++++
>  drivers/iio/industrialio-core.c         |  1 +
>  include/uapi/linux/iio/types.h          |  1 +
>  tools/iio/iio_event_monitor.c           |  1 +
>  4 files changed, 16 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> index bd6c3305dd2b..54e38ebb044d 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio
> +++ b/Documentation/ABI/testing/sysfs-bus-iio
> @@ -1755,6 +1755,19 @@ Description:
>  		measurement from channel Y. Units after application of scale and
>  		offset are milliamps.
>  
> +What:		/sys/bus/iio/devices/iio:deviceX/in_rot_partial_quaternion_raw
> +KernelVersion:	7.1
> +Contact:	linux-iio@xxxxxxxxxxxxxxx
> +Description:
> +		Raw value of {x, y, z} components of the quaternion vector. These
> +		components represent the axis about which a rotation occurs, and are
> +		subject to the following costraints:

s/costraints/constraints/

> +		- the quaternion vector is normalized, i.e. x^2 + y^2 + z^2 + w^2 = 1

Isn't this usually written with w first?

> +		- the rotation angle is within the [-180, 180] range, i.e. the w

Best to say the angle units. IIO standard unit for angle is radians.

> +		  component (which represents the amount of rotation) is non-negative
> +		These constraints allow the w value to be calculated from the other
> +		components: w = sqrt(1 - (x^2 + y^2 + z^2)).
> +
>  What:		/sys/.../iio:deviceX/in_energy_en
>  What:		/sys/.../iio:deviceX/in_distance_en
>  What:		/sys/.../iio:deviceX/in_velocity_sqrt(x^2+y^2+z^2)_en
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index 22eefd048ba9..792fbeb0dfa8 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -125,6 +125,7 @@ static const char * const iio_modifier_names[] = {
>  	[IIO_MOD_LIGHT_UVB] = "uvb",
>  	[IIO_MOD_LIGHT_DUV] = "duv",
>  	[IIO_MOD_QUATERNION] = "quaternion",
> +	[IIO_MOD_PARTIAL_QUATERNION] = "partial_quaternion",

We've found that using `_` in components of the attribute name makes it
difficult to parse since you have to know all component names to know
where the break between components is. Each time we add something with
an `_` means all parsers need to be updated to handle the new name.

In other components, we've just squashed the two words together without
any space or punctuation to get around this problem. I find that rather hard
to read though.

I wonder if we could propose something different going forward to do a better
job being consistent and readable. For example, use a `-` at the word break
in components that have more than one word in the name.

>  	[IIO_MOD_TEMP_AMBIENT] = "ambient",
>  	[IIO_MOD_TEMP_OBJECT] = "object",
>  	[IIO_MOD_NORTH_MAGN] = "from_north_magnetic",
> diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
> index 6d269b844271..c80ef7c1ed12 100644
> --- a/include/uapi/linux/iio/types.h
> +++ b/include/uapi/linux/iio/types.h
> @@ -77,6 +77,7 @@ enum iio_modifier {
>  	IIO_MOD_LIGHT_GREEN,
>  	IIO_MOD_LIGHT_BLUE,
>  	IIO_MOD_QUATERNION,
> +	IIO_MOD_PARTIAL_QUATERNION,

This is userspace API. We can't change the values of the enum
members after this, so new members have to be added at the end.

>  	IIO_MOD_TEMP_AMBIENT,
>  	IIO_MOD_TEMP_OBJECT,
>  	IIO_MOD_NORTH_MAGN,
> diff --git a/tools/iio/iio_event_monitor.c b/tools/iio/iio_event_monitor.c
> index 03ca33869ce8..db0fb75806ab 100644
> --- a/tools/iio/iio_event_monitor.c
> +++ b/tools/iio/iio_event_monitor.c
> @@ -113,6 +113,7 @@ static const char * const iio_modifier_names[] = {
>  	[IIO_MOD_LIGHT_UVB] = "uvb",
>  	[IIO_MOD_LIGHT_DUV] = "duv",
>  	[IIO_MOD_QUATERNION] = "quaternion",
> +	[IIO_MOD_PARTIAL_QUATERNION] = "partial_quaternion",
>  	[IIO_MOD_TEMP_AMBIENT] = "ambient",
>  	[IIO_MOD_TEMP_OBJECT] = "object",
>  	[IIO_MOD_NORTH_MAGN] = "from_north_magnetic",