Skip to content

DRAFT: add AD7173 family - #42

Draft
granquet wants to merge 1 commit into
analogdevicesinc:mainfrom
granquet:ad7173
Draft

DRAFT: add AD7173 family#42
granquet wants to merge 1 commit into
analogdevicesinc:mainfrom
granquet:ad7173

Conversation

@granquet

Copy link
Copy Markdown
  • AD4111 12 channel (8 voltage/4 current) +1 temperature

Pull Request Description

Please replace this with a detailed description and motivation of the changes.
You can tick the checkboxes below with an 'x' between square brackets or just check them after publishing the PR.
If this PR contains a breaking change, list dependent PRs and try to push all related PRs at the same time.

PR Type

  • Bug fix (change that fixes an issue)
  • New feature (change that adds new functionality)
  • Breaking change (has dependencies in other repos or will cause CI to fail)

PR Checklist

  • I have performed a self-review of the changes
  • I have commented my code, at least hard-to-understand parts
  • I have build all projects affected by the changes in this PR
  • I have tested in hardware affected projects, at the relevant boards
  • I have signed off all commits from this PR
  • I have updated the documentation by running /docs/source/reference_api/gen_sysobj_doc.m script locally, if applies.

* AD4111 12 channel (8 voltage/4 current) +1 temperature

Signed-off-by: Guillaume Ranquet <granquet@baylibre.com>
@CLAassistant

CLAassistant commented Nov 21, 2024

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

Comment thread +adi/+AD7173/Base.m
Comment thread +adi/+AD7173/Base.m
properties (Nontunable, Hidden)
Timeout = Inf
kernelBuffersCount = 1
dataTypeStr = 'int32'

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is int32 the best type available? most devices declare realbits to 24 and ad4113 is 16 bits

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For 16 bit resolution & less than that, dataTypeStr should be set to int16. For values higher than 16 (eg. 18/24 etc), the dataTypeStr has to be set to int32, which is the next value of powers of two.

Comment thread +adi/+AD7173/Base.m
%% Check Voltage Scale
function rValue = get.VoltageScale(obj)
if obj.ConnectedToDevice
rValue = obj.getAttributeDouble('voltage0', 'scale', obj.isOutput);

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the default of 'voltage0' might not exists depending on how the dtb is written, how to account for that? I see GetChannelAttrValue() exists in +Generic/Rx.m but I'm unsure how to get the chnID from the first enabled channel?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you have access to the hardware, the fastest way to get the channels names & attributes is running iio_info.

@granquet granquet Feb 12, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have access to the hardware, but if no voltage channels are declared in dts, there will be no voltage channels?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think they are named "voltage0" and so on in the dts. I think they are reffered to as "channels". I'm not very familiar with Linux, but if I'm not wrong, those namings are from iio.

Comment thread +adi/+AD7173/Base.m
Comment thread +adi/+AD4111/Rx.m
channel_names = {'voltage0','voltage1','voltage2','voltage3',...
'voltage4','voltage5','voltage6','voltage7','current0', ...
'current1','current2','current3','differential0', ...
'differential1','differential2','differential3', 'temp'};

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the channels will not be named differentialX but something like in_voltage3-voltage5_raw
I'm unsure how to translate this here? other devices in the toolbox seems to use differentialX, not sure how this works?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, as I mentioned above, if you can run iio_info it should ease the naming process. If you're using only the Linux drivers, you can take an example from previous similar scenarios.
In this PR there are both differential and non-differential parts.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes but, the number and naming of the differential channels depends on the device tree... and this is hardcoded... so should I hardcode all possible values in this PR!?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, if the values are hardcoded I think we should have separate files for each devicetree/ channels combinations.

@StancaPop

Copy link
Copy Markdown
Collaborator

Do we have any updates on this draft PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants