Add initial support and doc for ad4020 #12

apelete · 2023-09-29T14:41:36Z

This adds initial support and doc for AD4020 ADC.

Matlab implementation currently depends on AD40xx Linux driver work-in-progress implementation (analogdevicesinc/linux#2257).

This is also subject to a limitation in triggered buffer support in Linux driver at the moment (see https://github.com/adi-ses/sea-misc/pull/27#issuecomment-1740985035).

+adi/+AD4020/Rx.m

CLAassistant · 2023-10-05T05:13:42Z

All committers have signed the CLA.

mphalke · 2023-10-05T05:15:57Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

Apelete Seketeli seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@apelete please sign the CLA to get this PR accepted

apelete · 2023-10-09T13:14:23Z

@tfcollins AFAIK requested changes were implemented so far : any update on this ?

ribdp · 2023-10-09T17:44:11Z

Looks good to me. Just one thing - could you possibly run the gen_sysobj_doc.m and make a final push? The documentation should get updated using doc strings supplied in the AD4020 driver.

apelete · 2023-10-10T09:11:28Z

Looks good to me. Just one thing - could you possibly run the gen_sysobj_doc.m and make a final push? The documentation should get updated using doc strings supplied in the AD4020 driver.

Thanks for the quick reply : latest commit updates sysobjs.json as per your instructions.

ribdp · 2023-10-10T10:21:30Z

CI/gen_doc/docs/sysobjs.json

      },
      {
        "prop_name": "uri",
        "prop_title": "  URI - remote host URI",
        "prop_description": "Hostname or IP address of remote libIIO deviceHelp for adi.AD4630_24.Rx/uri is inherited from superclass matlabshared.libiio.base"
      }
    ]
+  },
+  {
+    "name": "adi.AD4020.Rx",


Unlike ad2s120, properties didn't get filled out for AD4020. So documentation wouldn't get generated.

Just to confirm did you run the gen_sysobj_doc.m with the most recent driver version checked out?

I noticed the difference with ad2s1210 but can't figure out why properties are not filled out for ad4020 : I'm running gen_sysobj_doc.m with the same Matlab version, just checking out the relevant branch for each device in my git workspace.

I tried to replicate the behavior locally. I cloned the repo, switched to this branch, and executed gen_sysobj_doc - I could see the properties appear correctly in sysobjs.json.

Is there another clone (perhaps with the early version of the driver or something) on the MATLAB path, that's taking precedence? That could explain why the properties don't show up.

Did the same on my side : cloned the repo in another directory and executed gen_sysobj_doc in that new directory with the same result and I still cannot see the properties in sysobjs.json for AD4020.

@ribdp Would you please consider pushing your sysobjs.json changes with the properties updated since this appears to be a setup issue on my side ?
I can't figure out why the properties do not show after executing gen_sysobj_doc.m in this case, even looking at MATLAB path for a probable cause.

@apelete Sure thing. I've pushed the changes now

ribdp

Looks good to me

apelete · 2023-10-11T11:04:57Z

@ribdp @tfcollins I rebased and signed all commits with GPG to get them verified, can you please merge this pull request into main branch ?
I am not able to do so through GitHub webui, getting the following error when clicking on "Rebase and merge" button :

Merge attempt failed

Base branch requires signed commits. Rebase merges cannot be automatically signed by GitHub

apelete · 2023-10-12T14:52:06Z

@ribdp @tfcollins There may be a merge issue when signing the commits with GPG according to documentation (https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification#signature-verification-for-rebase-and-merge) :

When using the Rebase and Merge option on a pull request, it's important to note that the commits in the head branch are added to the base branch without commit signature verification. When you use this option, GitHub creates a modified commit, using the data and content of the original commit. This means that GitHub didn't truly create this commit, and can't therefore sign it as a generic system user. GitHub doesn't have access to the committer's private signing keys, so it can't sign the commit on the user's behalf.

A workaround for this is to rebase and merge locally, and then push the changes to the pull request's base branch.

Do you want me to squash and merge all commits before you push the resulting single commit into the main branch to work around this issue ?

mphalke · 2023-10-13T04:59:25Z

@ribdp @tfcollins There may be a merge issue when signing the commits with GPG according to documentation (https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification#signature-verification-for-rebase-and-merge) :

When using the Rebase and Merge option on a pull request, it's important to note that the commits in the head branch are added to the base branch without commit signature verification. When you use this option, GitHub creates a modified commit, using the data and content of the original commit. This means that GitHub didn't truly create this commit, and can't therefore sign it as a generic system user. GitHub doesn't have access to the committer's private signing keys, so it can't sign the commit on the user's behalf.
A workaround for this is to rebase and merge locally, and then push the changes to the pull request's base branch.

Do you want me to squash and merge all commits before you push the resulting single commit into the main branch to work around this issue ?

@apelete We have added some branch protection rules and I think as a part of that, above error is thrown. In order to get changes merged properly, only admins should/can merge the PR. Also, we use fast-forward merge strategy for this, so that whole commit history is fetched from recent PR commits of source branch and put onto the top of target branch.
I see PR is approved now, so it can be merged by ADI team. @ribdp or @tfcollins I think this PR can be merged now.

Also another thing to look in future is to add part/project name prefix to the commits title.
e.g.
git commit -s -m "AD020: Title of the commit

Commit body
Commit body

This help us identify quickly for which project the change was made.

This adds initial support and doc for AD4020 ADC. Signed-off-by: Apelete Seketeli <[email protected]>

Signed-off-by: Apelete Seketeli <[email protected]>

apelete · 2023-10-13T14:34:47Z

@ribdp Same here, rebased on main branch and solved the conflicts, should be good for merging now.

apelete requested a review from ribdp September 29, 2023 14:41

ribdp requested a review from tfcollins October 2, 2023 15:58

ribdp reviewed Oct 4, 2023

View reviewed changes

+adi/+AD4020/Rx.m Show resolved Hide resolved

ribdp reviewed Oct 4, 2023

View reviewed changes

+adi/+AD4020/Rx.m Outdated Show resolved Hide resolved

ribdp mentioned this pull request Oct 10, 2023

Add initial support and doc for ad2s1210 #11

Merged

ribdp reviewed Oct 10, 2023

View reviewed changes

ribdp force-pushed the aseketeli/ad4020-support branch from e7e3703 to b8e990c Compare October 10, 2023 15:54

ribdp previously approved these changes Oct 10, 2023

View reviewed changes

tfcollins previously approved these changes Oct 10, 2023

View reviewed changes

apelete force-pushed the aseketeli/ad4020-support branch from b8e990c to fd66408 Compare October 11, 2023 11:02

apelete added 6 commits October 13, 2023 16:31

Add initial support and doc for ad4020

8a9c673

This adds initial support and doc for AD4020 ADC. Signed-off-by: Apelete Seketeli <[email protected]>

Remove unused property

3ce078c

Signed-off-by: Apelete Seketeli <[email protected]>

Add SamplesPerFrame property back

46d4632

Signed-off-by: Apelete Seketeli <[email protected]>

Remove unused FrameCount property

aa7fd92

Signed-off-by: Apelete Seketeli <[email protected]>

Remove read-only SamplingFrequency property

72ff4b4

Signed-off-by: Apelete Seketeli <[email protected]>

Update documentation using doc strings

6eaf1a5

Signed-off-by: Apelete Seketeli <[email protected]>

apelete dismissed stale reviews from ribdp and tfcollins via 6eaf1a5 October 13, 2023 14:33

apelete force-pushed the aseketeli/ad4020-support branch from fd66408 to 6eaf1a5 Compare October 13, 2023 14:33

ribdp merged commit b52c0ba into main Oct 13, 2023
2 checks passed

pamolloy deleted the aseketeli/ad4020-support branch October 13, 2023 15:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add initial support and doc for ad4020 #12

Add initial support and doc for ad4020 #12

apelete commented Sep 29, 2023

CLAassistant commented Oct 5, 2023 •

edited

Loading

mphalke commented Oct 5, 2023

apelete commented Oct 9, 2023

ribdp commented Oct 9, 2023

apelete commented Oct 10, 2023

ribdp Oct 10, 2023

apelete Oct 10, 2023

ribdp Oct 10, 2023

ribdp Oct 10, 2023

apelete Oct 10, 2023

apelete Oct 10, 2023

ribdp Oct 10, 2023

ribdp left a comment

apelete commented Oct 11, 2023

apelete commented Oct 12, 2023

mphalke commented Oct 13, 2023 •

edited

Loading

apelete commented Oct 13, 2023

Add initial support and doc for ad4020 #12

Add initial support and doc for ad4020 #12

Conversation

apelete commented Sep 29, 2023

CLAassistant commented Oct 5, 2023 • edited Loading

mphalke commented Oct 5, 2023

apelete commented Oct 9, 2023

ribdp commented Oct 9, 2023

apelete commented Oct 10, 2023

ribdp Oct 10, 2023

Choose a reason for hiding this comment

apelete Oct 10, 2023

Choose a reason for hiding this comment

ribdp Oct 10, 2023

Choose a reason for hiding this comment

ribdp Oct 10, 2023

Choose a reason for hiding this comment

apelete Oct 10, 2023

Choose a reason for hiding this comment

apelete Oct 10, 2023

Choose a reason for hiding this comment

ribdp Oct 10, 2023

Choose a reason for hiding this comment

ribdp left a comment

Choose a reason for hiding this comment

apelete commented Oct 11, 2023

apelete commented Oct 12, 2023

mphalke commented Oct 13, 2023 • edited Loading

apelete commented Oct 13, 2023

CLAassistant commented Oct 5, 2023 •

edited

Loading

mphalke commented Oct 13, 2023 •

edited

Loading