labgrid
labgrid copied to clipboard
YKUSH set pykush in requierments and make available for remote access
Description
This pull requests has two important changes.
-
The ykush pypip implementation [2] lacks basic functionality like for example connect to several Yepkits via the serial number. Make sure to use the official [2] pykush implementation by adding the dependency in the requierements.txt
-
Make YKUSH accessible through remote access. With this patch we can export the access to the YKUSH and toggle the power via the labgrid-client interface.
[1] https://github.com/Yepkit/pykush [2] https://github.com/alvarop/pykush/
Checklist
- [ ] Documentation for the feature
- [ ] Tests for the feature
- [ ] The arguments and description in doc/configuration.rst have been updated
- [ ] Add a section on how to use the feature to doc/usage.rst
- [ ] Add a section on how to use the feature to doc/development.rst
- [x] CHANGES.rst has been updated
- [x] PR has been tested
- [ ] Man pages have been regenerated
Codecov Report
Merging #606 (6ceb945) into master (02d5b1b) will increase coverage by
0.2%
. The diff coverage is55.8%
.
@@ Coverage Diff @@
## master #606 +/- ##
========================================
+ Coverage 58.7% 59.0% +0.2%
========================================
Files 128 123 -5
Lines 9195 8716 -479
========================================
- Hits 5400 5143 -257
+ Misses 3795 3573 -222
Impacted Files | Coverage Δ | |
---|---|---|
labgrid/remote/client.py | 47.1% <0.0%> (+0.2%) |
:arrow_up: |
labgrid/resource/udev.py | 47.8% <66.6%> (+0.5%) |
:arrow_up: |
labgrid/remote/exporter.py | 63.9% <71.4%> (+1.6%) |
:arrow_up: |
labgrid/resource/remote.py | 82.0% <75.0%> (-0.1%) |
:arrow_down: |
labgrid/driver/powerdriver.py | 67.8% <100.0%> (ø) |
|
labgrid/resource/__init__.py | 100.0% <100.0%> (ø) |
|
labgrid/target.py | 89.1% <0.0%> (-3.4%) |
:arrow_down: |
labgrid/factory.py | 84.6% <0.0%> (-1.7%) |
:arrow_down: |
labgrid/config.py | 80.2% <0.0%> (-1.6%) |
:arrow_down: |
labgrid/driver/flashromdriver.py | 63.8% <0.0%> (-1.4%) |
:arrow_down: |
... and 30 more |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 02d5b1b...cd1b4c9. Read the comment docs.
In the medium term, I'd like to replace labgrid's powerdrivers with pdudaemon, which also has support for YKUSH (at least one of the versions). @mattface indicated that the approach to simplify it and support blocking requests (https://github.com/pdudaemon/pdudaemon/pull/78) can continue.
In the medium term, I'd like to replace labgrid's powerdrivers with pdudaemon, which also has support for YKUSH (at least one of the versions). @mattface indicated that the approach to simplify it and support blocking requests (pdudaemon/pdudaemon#78) can continue.
Does this mean you want me to abandon this pull request? I'd prefer to go on with this to get something working in the short term, then we can move that to pdudaemon in the future.
Does this mean you want me to abandon this pull request? I'd prefer to go on with this to get something working in the short term, then we can move that to pdudaemon in the future.
Besides the missing maintenance, what are the issues you have with https://github.com/Yepkit/pykush/? Just the missing remote support? I took a short look at the ykushcmd implementation and it seemed to have a lot of complexity for something that should be relatively simple. ;)
If you can solve the inconsistency between matching on serial number vs. matching on the bus topology, I'd merge the PR, as it would allow us to access them remotely. I'd suspect though, that finishing that pdudaemon PR may be easier to do.
Does this mean you want me to abandon this pull request? I'd prefer to go on with this to get something working in the short term, then we can move that to pdudaemon in the future.
Besides the missing maintenance, what are the issues you have with https://github.com/Yepkit/pykush/? Just the missing remote support? I took a short look at the ykushcmd implementation and it seemed to have a lot of complexity for something that should be relatively simple. ;)
The problem with pykush is that the pip version has changed and doesn't work with the labgrid implementation anymore. So you would need to install pykush from https://github.com/Yepkit/pykush instead from pip.
If you can solve the inconsistency between matching on serial number vs. matching on the bus topology, I'd merge the PR, as it would allow us to access them remotely. I'd suspect though, that finishing that pdudaemon PR may be easier to do.
Could we use ID_SERIAL_SHORT from udevadm info for matching in udev?
It shows me: E: ID_SERIAL_SHORT=YKC6071
Does this mean you want me to abandon this pull request? I'd prefer to go on with this to get something working in the short term, then we can move that to pdudaemon in the future.
Besides the missing maintenance, what are the issues you have with https://github.com/Yepkit/pykush/? Just the missing remote support? I took a short look at the ykushcmd implementation and it seemed to have a lot of complexity for something that should be relatively simple. ;)
The problem with pykush is that the pip version has changed and doesn't work with the labgrid implementation anymore. So you would need to install pykush from https://github.com/Yepkit/pykush instead from pip.
I've only ever tried it with the version from git, which is also referred to in the source. We could add a git entry for it in a requirements.txt.
If you can solve the inconsistency between matching on serial number vs. matching on the bus topology, I'd merge the PR, as it would allow us to access them remotely. I'd suspect though, that finishing that pdudaemon PR may be easier to do.
Could we use ID_SERIAL_SHORT from udevadm info for matching in udev?
It shows me: E: ID_SERIAL_SHORT=YKC6071
I don't have a ykush available for testing. Is that the same serial as used with the command line tool?
Does this mean you want me to abandon this pull request? I'd prefer to go on with this to get something working in the short term, then we can move that to pdudaemon in the future.
Besides the missing maintenance, what are the issues you have with https://github.com/Yepkit/pykush/? Just the missing remote support? I took a short look at the ykushcmd implementation and it seemed to have a lot of complexity for something that should be relatively simple. ;)
The problem with pykush is that the pip version has changed and doesn't work with the labgrid implementation anymore. So you would need to install pykush from https://github.com/Yepkit/pykush instead from pip.
I've only ever tried it with the version from git, which is also referred to in the source. We could add a git entry for it in a requirements.txt.
That would work as well. I can find out how to do that.
If you can solve the inconsistency between matching on serial number vs. matching on the bus topology, I'd merge the PR, as it would allow us to access them remotely. I'd suspect though, that finishing that pdudaemon PR may be easier to do.
Could we use ID_SERIAL_SHORT from udevadm info for matching in udev? It shows me: E: ID_SERIAL_SHORT=YKC6071
I don't have a ykush available for testing. Is that the same serial as used with the command line tool?
Yes:
# ykushcmd -l
Attached YKUSH Boards
---------------------
YKUSH device found with Serial Number: YKC6071
Manufacturer: Yepkit Lda.
Product: YKUSH
I'm not sure how I'll have to implement this. Using the existing ID_VENDOR_ID and ID_MODEL_ID and add ID_SERIAL_SHORT or just use ID_SERIAL_SHORT?
Ok the pull request now add the pykush dependency to the requierements file and matches against the serial number of the YKUSH.
@jluebbe please review again :)
I have pushed a commit onto your branch which should fix the travis failure.
I have pushed a commit onto your branch which should fix the travis failure.
Thanks! This fixed the issue in travis but not yet in docker/build.sh
@jluebbe any comments on this? It's been a while since I pushed to code for review :)
@mbgg I guess most of the issues this PR tried to solve are now solved by #1143. Is there something left? If not, can we close this?