channel type: Improve code readability for channel type

[RubenNatanael]

PR Description

Currently, the channel type is stored as enum iio_chan_type, even when it represents a hwmon_chan_type. By changing this field to int, we improve code readability and flexibility. This change also enables the implementation of validation checks, such as in hwmon_channel_get_type(), which now issues a warning if a hwmon function is incorrectly called for a non-hwmon channel and vice versa.

• Issue: Incorrect mapping from hwmon sensor type into iio sensor type #1401

PR Type

• Bug fix (a change that fixes an issue)
• New feature (a change that adds new functionality)
• Breaking change (a change that affects other repos or cause CIs to fail)

PR Checklist

• I have conducted a self-review of my own code changes
• I have commented new code, particularly complex or unclear areas
• I have checked that I did not introduce new warnings or errors (CI output)
• I have checked that components that use libiio did not get broken
• I have updated the documentation accordingly (GitHub Pages, READMEs, etc)

[nunojsa]

I'm not sure I agree with this change, It seems like unneeded churn to me. It is fair to assume that users will call hwmon_channel_get_type() on an hwmon device. If not, they need to fix their app anyways! So I would either:

1. Leave as-is;
2. Or be strict about it. Meaning, return an error in case we try to get an IIO channel from an hwmon device or vice-versa.

Honestly I would probably go with 1. For 2, we might need to change the API a bit...

Having said the above, we should have introduced different types for hwmon devices. Yeah, it would have been more complicated but we would not have these discussions.

[nunojsa]

well, we give a warning but prevent nothing... IOW, we continue to do as before.

[RubenNatanael]

I agree. Like I said in the issue, this doesn't stop the error from happening, it's just a 'hint' for developers who assume that the member enum iio_chan_type type in struct iio_channel can't be an hwmon_chan_type. That said, I'm not entirely sure about this change either, so I'd like to hear other opinions.

[rgetz]

I'm not sure I agree with this change, It seems like unneeded churn to me.

Ack and agree - it's not only unneeded - but also will break things. We want to have people treat hwmon devices as IIO devices. a Voltage is a voltage. It does not matter if the backing device is implemented in the hwmon subsystem or the iio one.

[damijans]

Hi all and thanks for all responses. I didn't expect the #1401 will lead to so quick reactions. It doesn't happen so often in Open Source community.

I believe we all agree that actually there is nothing wrong in the current implementation, if you use it correctly it works as expected. The only problem is about confusion faced to developers who are based on reading the code and not documentation. And the fact is that the code is the best documentation. Unfortunately, if you see iio_chan_type type in iio_channel struct (yes, I know it is private structure) then you simply mean that it represent iio channel type and not hwmon type. And we know these two types are not compatible. It true the voltage is a voltage (as mentioned above) and temperature is temperature, but there exist types which can not be simply converted

The fact is that in existing implementation developer is confused when seeing iio_chan_type type attribute which semantically in certain cases means something else. Proposed change in this thread (change attribute into opaque integer) leads developer to think about "hey, this type is opaque, I should do something about that". Proposed change about having a warning if incorrect function is used is also good direction of using the library properly. The alternative, to make the code more self explicable, could also be usage of union instead of integer.

Anyway, whatever decision will be made, I am glad of having such quick responds, thank you.