-
Notifications
You must be signed in to change notification settings - Fork 8.1k
boards: shields: Add Adafruit DS2484 1-wire shield #95215
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
boards: shields: Add Adafruit DS2484 1-wire shield #95215
Conversation
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.
Pull Request Overview
This PR adds support for the Adafruit DS2484 I2C to 1-Wire Bus Adapter as a Zephyr shield, enabling 1-wire device communication through I2C connectivity.
- Introduces a new shield configuration for the Adafruit DS2484 adapter
- Refactors test harness configuration to move from common to individual test level
- Adds shield-specific test case with appropriate platform configuration
Reviewed Changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
boards/shields/adafruit_ds2484/shield.yml | Shield metadata defining name, vendor, and supported features |
boards/shields/adafruit_ds2484/doc/index.rst | Complete documentation with overview, requirements, pin assignments, and usage examples |
boards/shields/adafruit_ds2484/adafruit_ds2484.overlay | Device tree overlay configuring the DS2484 I2C device and w1 node aliases |
boards/shields/adafruit_ds2484/Kconfig.shield | Kconfig shield selection configuration |
samples/drivers/w1/scanner/sample.yaml | Test harness refactoring and addition of shield-specific test case |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
common: | ||
harness: console | ||
tags: |
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.
what is the issue with the console harness and your shield?
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.
Maybe I have misunderstood how this works. Ideally I would like to test on hardware (an actual shield). However now I at least would like to have a build-only test. Will that give any output on a console? Any help is appreciated.
It seems to work locally if I change back to the original "common" setting and add
sample.drivers.w1.scanner.shields:
platform_allow:
- adafruit_qt_py_rp2040/rp2040
extra_args:
- platform:adafruit_qt_py_rp2040/rp2040:SHIELD="adafruit_ds2484"
harness_config:
type: one_line
Is that a better way?
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.
Two minor notes.
2789d27
to
046edab
Compare
046edab
to
04c885d
Compare
@str4t0m I modified the sample.yaml to
Then I can compile-test the overlay file locally with:
and the resulting twister.json contains:
|
When I connect the shield to an adafruit_qt_py_rp2040, manually set the board to bootloader mode, and run:
the result is a passing device test:
|
So what is the consensus @str4t0m @JarmouniA @kartben ? |
In my opinion, no, the problem of testing with shields is general, not specific to this PR. |
If you have a chance could you please add the fixture as discussed on weekend above.
|
04c885d
to
b84c402
Compare
Rebased on main to fix CI issues. |
@JarmouniA and @str4t0m How do we proceed with this PR? Can it be merged now and the fixture be added later? |
Product photo from https://learn.adafruit.com/assets/130701 with license CC BY-SA 3.0 Tested with command mentioned in the index.rst page. Signed-off-by: Jonas Berg <[email protected]>
|
@str4t0m OK, I did not understand what to do previously. Now I added the fixture line. Can you please have a look to see that it is correct? |
Great, thanks! Yes that what I meant. |
Product photo from https://learn.adafruit.com/assets/130701 with license CC BY-SA 3.0
Tested with command mentioned in the index.rst page.