-
Notifications
You must be signed in to change notification settings - Fork 2.1k
drivers/gp8xxx: add initial support #21182
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.
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
Conversation
8643628 to
c59968c
Compare
c59968c to
6fe9152
Compare
6fe9152 to
5fac517
Compare
b5824f6 to
c13315e
Compare
|
I took the liberty of rebasing this PR and adapt it to meet the conventions since last push. Tested this once more using the SLSTK3401A with the GP8302 + GP8403 combination. |
| { | ||
| assert(dev->params.info->type == GP8XXX_INFO_TYPE_VDAC); | ||
|
|
||
| /* only the 5 V/10 V models have a customizable range */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it make sense to return an error when the range wasn't actually set because the device doesn't support it? -ENOSUP for example?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I decided that wrong use would fall in the use of programming errors, and that an assert would be sufficient alternative to another return value or making it type safer.
dist/tools/doccheck/exclude_simple
Outdated
| warning: Member GP8211S_INFO_CHANNELS (macro definition) of file gp8xxx_info.h is not documented. | ||
| warning: Member GP8211S_INFO_RANGE (macro definition) of file gp8xxx_info.h is not documented. | ||
| warning: Member GP8211S_INFO_RESOLUTION (macro definition) of file gp8xxx_info.h is not documented. | ||
| warning: Member GP8302_INFO_CHANNELS (macro definition) of file gp8xxx_info.h is not documented. | ||
| warning: Member GP8302_INFO_RANGE (macro definition) of file gp8xxx_info.h is not documented. | ||
| warning: Member GP8302_INFO_RESOLUTION (macro definition) of file gp8xxx_info.h is not documented. | ||
| warning: Member GP8403_INFO_CHANNELS (macro definition) of file gp8xxx_info.h is not documented. | ||
| warning: Member GP8403_INFO_RANGE (macro definition) of file gp8xxx_info.h is not documented. | ||
| warning: Member GP8403_INFO_RESOLUTION (macro definition) of file gp8xxx_info.h is not documented. | ||
| warning: Member GP8413_INFO_CHANNELS (macro definition) of file gp8xxx_info.h is not documented. | ||
| warning: Member GP8413_INFO_RANGE (macro definition) of file gp8xxx_info.h is not documented. | ||
| warning: Member GP8413_INFO_RESOLUTION (macro definition) of file gp8xxx_info.h is not documented. | ||
| warning: Member GP8503_INFO_CHANNELS (macro definition) of file gp8xxx_info.h is not documented. | ||
| warning: Member GP8503_INFO_RANGE (macro definition) of file gp8xxx_info.h is not documented. | ||
| warning: Member GP8503_INFO_RESOLUTION (macro definition) of file gp8xxx_info.h is not documented. | ||
| warning: Member GP8512_INFO_CHANNELS (macro definition) of file gp8xxx_info.h is not documented. | ||
| warning: Member GP8512_INFO_RANGE (macro definition) of file gp8xxx_info.h is not documented. | ||
| warning: Member GP8512_INFO_RESOLUTION (macro definition) of file gp8xxx_info.h is not documented. | ||
| warning: Member GP8XXX_REG_CHANNEL (macro definition) of file gp8xxx_constants.h is not documented. | ||
| warning: Member GP8XXX_REG_OUTPUT (macro definition) of file gp8xxx_constants.h is not documented. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would much prefer if you instead documented the macros.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Documented them, so this commit will be removed.
| ifneq (,$(filter gp8xxx,$(DRIVERS))) | ||
| USEMODULE += gp8403 | ||
| USEMODULE += gp8302 | ||
| endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would this also work when selecting gp8512 module in the test application?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested this by only selecting gp8512 AND including gp8512, which compiled fine.
|
Thanks @crasbe, I will take care of the comments tonight (hopefully). |
1f60595 to
9a4f7cc
Compare
|
@crasbe I have addressed all comments. I commented on the ones still unresolved, to check if you agree. Did another round of testing with the SLSTK3401a (as in the videos) and it still works. |
|
@crasbe anything I need to address, or can I squash? |
crasbe
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some super small nitpicks, you can squash the changes directly I think.
9a4f7cc to
ee3150a
Compare
|
The test needs a |
|
Here you go: |
To add support for voltage and current DACs, add additional SAUL actuators.
The Guestgood GP8xxx I2C DACs are easy-to-use voltage and current DACs with standardized output ranges. All supported variants have very similar I2C protocol. This commit adds support for a generic driver (gp8xxx) and uses a information structure to record capabilities. Multiple driver instances can work simultaneously on the same I2C bus. SAUL support is also added.
These two drivers ensure that a VDAC and IDAC capabilities are included.
ee3150a to
47a53e8
Compare
|
Thanks. I took the liberty to immediately squash that into the commit for the test application. |
|
Thank you @crasbe for the review and help to get this merged! |
Contribution description
This adds support for the Guestgood GP8xxx I2C voltage and current DACs. The following variations are supported.
Although Guestgood is Chinese and doesn't offer English datasheets, small and well-documented development boards are produced by DFRobot, which are distributed by several known distributors such as Mouser and Digikey.
The 0-10V VDACs had my particular interest, because several home automation appliances can be controlled using 0-10V signals. These single-chip solutions are very ideal for this purpose.
SAUL was extended to support voltage and current actuators.
Testing procedure
A test application is provided in
tests/drivers/gp8xxx. By default, it will select the GP8403, but additional actuators can be specified usingUSEMODULE. I tested the GP8403 and GP8302 only, but the others will probably work out of the box (my driver is largely based on DFRobot's Arduino version).SAUL integration can be tested with
examples/basic/saul. Note that when using this application, the arguments are parsed as Volts or Amperes. Unfortunately, the SAUL shell does not support fractional numbers, so testing the full range is not really possible for the 0-25 mA IDAC.I have created three videos to show the driver in action:
tests/drivers/gp8xxxwith only GP8403tests/drivers/gp8xxxwith both GP8302 + GP8403examples/basic/saulwith both GP8302 + GP8403My test setup:
Issues/PRs references
None