protocol/driver: add DigitalInputProtocol, GpioDigitalInputDriver - #1458
protocol/driver: add DigitalInputProtocol, GpioDigitalInputDriver#1458flxzt wants to merge 1 commit into
DigitalInputProtocol, GpioDigitalInputDriver#1458Conversation
|
I think the way I implemented the behaviour of the client doesn't work well - if I'll revert to the previous behaviour where |
|
can anyone review this? |
this adds a new protocol `DigitalInputProtocol` and a GPIO driver that implements it. The `DigitalOutputProtocol` ABC now also inherits it, ensuring it can be used in all existing driver implementations. By seperating the input from output protocol it is possible to make sure the GPIO line is configured in direction `input` in sysfs, which avoids possible shorts when reading out digital states from DUT's. Otherwise, one would have to rely on the circuitry of the GPIO line to prevent that. Signed-off-by: Felix Zwettler <Felix.Zwettler@duagon.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1458 +/- ##
========================================
- Coverage 46.0% 45.8% -0.2%
========================================
Files 180 182 +2
Lines 14462 14529 +67
========================================
+ Hits 6654 6668 +14
- Misses 7808 7861 +53
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
Emantor
left a comment
Emantor
left a comment
There was a problem hiding this comment.
We should not need an additional agent that duplicates the code, the existing agent should be extended to support the direction setting as well.
If possible it would also be great if the new gpiochip based sysfs interface that is not deprecated could be used.
| drv.set(False) | ||
| if not drv: | ||
| raise UserError("target has no compatible resource available") | ||
|
|
There was a problem hiding this comment.
Why is the get() action removed here?
|
Hey @Emantor, I took a look at this PR and would like to open a new PR based on it, with the review feedback addressed since its 2 years old and we need it. I leave the gpiochip/libgpiod work as future work for now. I agree that it would be nice to support the newer non-deprecated GPIO interface, but that seems like a larger design topic and should not block a smaller sysfs-based digital input PR. |
|
Superseded by #1935 |
4 participants
Description
this adds a new protocol
DigitalInputProtocoland a GPIO driver that implements it.The
DigitalOutputProtocolABC now also inherits it, ensuring it can be used in all existing driver implementations.Additionally the client now automatically uses
DigitalInputProtocolwhen the command isio get.By seperating the input from output protocol it is possible to make sure the GPIO line is configured in direction
inputin sysfs, which avoids shorts when reading out digital states from DUT's. Otherwise, one would have to rely on the circuitry of the GPIO line.Tested on two Raspberry Pi's as exporter and DUT.
Note:
the
sysfsgpioin.pypart duplicates most of thesysfsgpio.pycode but because of the way this code is executed by agentwrapper I thought it was cleaner to separate it into a new file.Checklist
closes #1457