adt7420: Adding Linux support #668
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Macy Libed <[email protected]>
Signed-off-by: Macy Libed <[email protected]>
Signed-off-by: Macy Libed <[email protected]>
|
Hi @tfcollins this PR is ready for review :> thanks |
tfcollins
reviewed
May 22, 2025
Comment on lines
+25
to
+40
| for ch in self._ctrl.channels: | ||
| name = ch._id | ||
|
|
||
| self._rx_channel_names.append(name) | ||
| self.channel.append(name) | ||
|
|
||
| if name == "temp": | ||
| # no-OS | ||
| setattr(self, name, self.channel_temp(self._ctrl, name)) | ||
| self.temp = self.channel_temp(self._ctrl, "temp") | ||
| elif name == "temp1": | ||
| # Linux | ||
| setattr(self, name, self.channel_temp1(self._ctrl, name)) | ||
| self.temp = self.channel_temp1(self._ctrl, "temp1") | ||
| else: | ||
| raise Exception(f"Unsupported: {name}") |
There was a problem hiding this comment.
alittle confused on this loop. Do all the channels have the same name? Is there more than one channel on the device? Maybe an emulated context will help
tfcollins
reviewed
May 22, 2025
Comment on lines
+93
to
+144
|
|
||
| class channel_temp1(attribute): | ||
| """ADT7420 Channel for Linux""" | ||
|
|
||
| def __init__(self, ctrl, channel_name): | ||
| self._ctrl = ctrl | ||
| self.name = channel_name | ||
|
|
||
| @property | ||
| def temp_val(self): | ||
| """ ADT7420 Channel Temperature Value """ | ||
| return self._get_iio_attr(self.name, "input", False) | ||
|
|
||
| @property | ||
| def temp_max(self): | ||
| """ ADT7420 Channel Max Temperature """ | ||
| return self._get_iio_attr(self.name, "max", False) | ||
|
|
||
| @temp_max.setter | ||
| def temp_max(self, value): | ||
| """ ADT7420 Channel Max Temperature """ | ||
| return self._set_iio_attr(self.name, "max", False, value) | ||
|
|
||
| @property | ||
| def temp_min(self): | ||
| """ ADT7420 Channel Min Temperature """ | ||
| return self._get_iio_attr(self.name, "min", False) | ||
|
|
||
| @temp_min.setter | ||
| def temp_min(self, value): | ||
| """ ADT7420 Channel Min Temperature """ | ||
| return self._set_iio_attr(self.name, "min", False, value) | ||
|
|
||
| @property | ||
| def temp_crit(self): | ||
| """ ADT7420 Channel Critical Temperature """ | ||
| return self._get_iio_attr(self.name, "crit", False) | ||
|
|
||
| @temp_crit.setter | ||
| def temp_crit(self, value): | ||
| """ ADT7420 Channel Critical Temperature """ | ||
| return self._set_iio_attr(self.name, "crit", False, value) | ||
|
|
||
| @property | ||
| def temp_hyst(self): | ||
| """ ADT7420 Channel Hysteresis Temperature """ | ||
| return self._get_iio_attr(self.name, "max_hyst", False) | ||
|
|
||
| @temp_hyst.setter | ||
| def temp_hyst(self, value): | ||
| """ ADT7420 Channel Hysteresis Temperature """ | ||
| return self._set_iio_attr(self.name, "max_hyst", False, value) |
There was a problem hiding this comment.
So I think just the channel names are different here. Can you instead do this inline to the properties like so:
def __init__(self, ctrl, channel_name, is_linux):
self._ctrl = ctrl
self.name = channel_name
self.is_linux = is_linux
@property
def temp_val(self):
""" ADT7420 Channel Temperature Value """
pname = "input" if self.is_linux else "temp"
return self._get_iio_attr(self.name, pname, False)
This will reduce the change by 4x and will be a bit easier to understand
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Added Linux support for ADT7420 by handling Linux-specific IIO attributes (input, max, min, crit, max_hyst). It can now auto-detect whether the platform is no-OS or Linux based on the channel name (temp for no-OS, temp1 for Linux).
Fixes # (issue)
Type of change
Please delete options that are not relevant.
How has this been tested?
Tested on actual hardware using the board example from pyadi-iio. The driver successfully reads and sets attribute values, confirming functionality on both no-OS and Linux setups.
Documentation
If this is a new feature or example please mention or link any documentation. All new hardware interface classes require documentation.
Checklist: