Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Adpd2140 get sensor #313

Merged
merged 3 commits into from
Apr 5, 2022
Merged

Adpd2140 get sensor #313

merged 3 commits into from
Apr 5, 2022

Conversation

adrimbarean
Copy link
Contributor

@adrimbarean adrimbarean commented Mar 8, 2022 •
edited
Loading

Description

Add adpd1080 pyadi-iio class, basic example and gesture sensor/theremin example.

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

How has this been tested?

Class and examples tested with hardware system. The first and second pins of the PMOD are not connected to the board by default in rev A and a wire needs to connect pin 1 of the PMOD to IO13 of the ADICUP3029.

  • Test 1: Change the com in the main function of the theremin module and run it with Python 3. Wait a few seconds (about 5) to calibrate and place hand above the PMOD. After keeping it for up to 6 seconds move it away. The program should detect the way of the movement.

Test Configuration:

  • Hardware: ADICUP3029 + CN0569
  • OS: Windows (Linux should be compatible)

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have signed off all commits and they contain "Signed-off by: "
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@adrimbarean adrimbarean self-assigned this Mar 8, 2022
@tfcollins tfcollins self-requested a review March 8, 2022 19:24
tfcollins
tfcollins reviewed Mar 8, 2022
View reviewed changes
adi/adpd1080.py Outdated
def rx(self):
buff = super().rx()
del self._rx__rxbuf
return buff
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you really need to delete the buffer between calls?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was done because in an application we needed to read a few samples and then take the average and write it into the offset register of the device. But writes to the device did not work anymore after buffer reads, unless the buffer was deleted first.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm that seems really strange. Is this a bug in the driver or the desired behavior?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know, exactly. I experienced an issue a while ago and @scuciurean helped me with this workaround. I think he experienced the issue as well.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I can leave it up to you and @scuciurean about if this is proper. Just let me know and we'll merge.

tfcollins
tfcollins reviewed Mar 8, 2022
View reviewed changes
adi/adpd1080.py Outdated Show resolved Hide resolved
@adrimbarean adrimbarean requested a review from a team March 9, 2022 08:29
@adrimbarean adrimbarean force-pushed the adpd2140_get_sensor branch from 228e938 to bef7256 Compare March 14, 2022 09:12
@adrimbarean
Copy link
Contributor Author

V1:

  • move sample rate to device properties.

@adrimbarean adrimbarean force-pushed the adpd2140_get_sensor branch 4 times, most recently from 610b725 to 155c02f Compare March 14, 2022 14:03
@adrimbarean
Copy link
Contributor Author

V2:

  • solved auto checking complaints (the remaining one is unrelated to this PR and solved here)

@adrimbarean adrimbarean force-pushed the adpd2140_get_sensor branch from 155c02f to dc55a68 Compare March 14, 2022 15:22
@adrimbarean
Copy link
Contributor Author

V3:

  • rebase on current master.

@adrimbarean adrimbarean force-pushed the adpd2140_get_sensor branch 2 times, most recently from 41fc252 to 907b1e7 Compare March 15, 2022 08:00
@adrimbarean
adi:adpd1080: initial commit
702bcc4 
Add pyadi-iio class for the adpd1080 device.

Signed-off-by: Andrei Drimbarean <[email protected]>
@adrimbarean adrimbarean force-pushed the adpd2140_get_sensor branch from 907b1e7 to 9da4e58 Compare March 15, 2022 08:46
@adrimbarean
Copy link
Contributor Author

Codacy complaints that there should be no blank lines before class docstring, but there are none. Seems like a false positive.

@adrimbarean adrimbarean force-pushed the adpd2140_get_sensor branch from 9da4e58 to 7ddca2a Compare March 28, 2022 14:38
@adrimbarean
Copy link
Contributor Author

V4:

  • remove unnecessary timeout assignation.

@adrimbarean adrimbarean force-pushed the adpd2140_get_sensor branch 2 times, most recently from b87703d to 862193c Compare April 5, 2022 11:54
@adrimbarean
Copy link
Contributor Author

V5:

  • comment code needed for workaround; to be uncommented after workaround is done;
  • re-apply black.

scuciurean
scuciurean reviewed Apr 5, 2022
View reviewed changes
examples/adpd1080.py Outdated
adpd1080 = adi.adpd1080(uri="serial:COM23")
adpd1080.rx_buffer_size = 40
# commented on adicup3029 platform, may work on others
# todo: undomment when uart issue on adicup is fixed
Copy link

Choose a reason for hiding this comment

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

Maybe we can remove this type of comments and add them as issues or tickets

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@scuciurean
Copy link

Apart from the 2 code analysis issues, it looks good to me.

@adrimbarean
Copy link
Contributor Author

Apart from the 2 code analysis issues, it looks good to me.

These seem like false positives to me. The issue is that blank lines are not allowed before class docstring, but there are no blank lines here.

@adrimbarean
examples:adpd1080: add adpd1080 example
3f26273 
Add an example of instantiating the ADPD1080 class and reading one sample
from every channel using the buffer.

Signed-off-by: Andrei Drimbarean <[email protected]>
@adrimbarean adrimbarean force-pushed the adpd2140_get_sensor branch from 862193c to 0137eb3 Compare April 5, 2022 13:41
@adrimbarean
Copy link
Contributor Author

adrimbarean commented Apr 5, 2022 •
edited
Loading

V6:

  • remove todo comments in favor of an issue to track workarounds;

@adrimbarean
examples:cn0569: gesture sensor example
8c1d3b0 
Initial commit for the gesture sensor/theremin example for the
ADPD1080/ADPD2140 PMOD.

Signed-off-by: Andrei Drimbarean <[email protected]>
@adrimbarean adrimbarean force-pushed the adpd2140_get_sensor branch from 0137eb3 to 8c1d3b0 Compare April 5, 2022 13:51
@tfcollins tfcollins merged commit 9c60f19 into analogdevicesinc:master Apr 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@scuciurean scuciurean scuciurean left review comments

@tfcollins tfcollins tfcollins left review comments

@buha buha buha approved these changes

Assignees

@adrimbarean adrimbarean

Labels
New Part
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@adrimbarean @scuciurean @tfcollins @buha