-
Notifications
You must be signed in to change notification settings - Fork 72
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
BF APT Piezo Motor Actuator Multi Channel support #373
base: main
Are you sure you want to change the base?
Conversation
Hmm, I don't see exactly what is wrong here. Some dependency that was updated? Fails locally as well when I recreate the tox environment, however, I can't seem to figure out which dependency it would be, since the CI just successfully ran two days ago. Any ideas @scasagrande? |
Yeah let me take a look... |
It looks like This might be a |
Thanks for looking into this. I will need some more time for the PR anyway to think it better through, next couple of weeks will be really busy... |
@trappitsch can you update your branch with latest |
Yey, the tests are running through smoothly now. Thanks for the fix @scasagrande. Now I just need to find the time to work on this more... it's a fun issue though :) |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #373 +/- ##
==========================================
- Coverage 99.11% 99.10% -0.01%
==========================================
Files 89 89
Lines 8962 8975 +13
==========================================
+ Hits 8883 8895 +12
- Misses 79 80 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
My apologies, it has been a while. @GarbatyGrabarz I have no idea if this is still of interest to you - I hope it is. I tried my hands on another fix for #372:
This is some try to get a handle on this mess, but since I don't have the hardware, it is difficult to say if this is a working solution. Any advise would be highly appreciated. |
Thanks for the fix! Unfortunately I am not able to verify it as currently I have no access to the device. I will let you know as soon as I get the access. Let's play a long game with this fix ;) |
Thanks @GarbatyGrabarz, that sounds great for me! Looking forward to the check and happy to work on it more if this doesn't work. |
This BF is based on the bug report by @GarbatyGrabarz in issue #372. Multiple channels, e.g., for the KIM101, are incorrectly addressed. The following channel number convention, as defined in APT manual, is now implemented:
Tests were extended such that the correct behavior is now tested for by checking - where required - using all 4 channels.