-
Notifications
You must be signed in to change notification settings - Fork 82
Additional support for PicoScope 4444 #151
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
Enabled Bandwidth Filtering for PicoScope 4444
|
Hello @rascustoms! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2019-10-02 14:11:42 UTC |
|
Looks good. Can you please fix the flake8 and pep8 errors. Since we have no other tests, these kinda act as a minimum barrier of protection. |
|
i guess there are some new flake8 errors that poped up recently. fixing them in #152 |
|
No problems on the pep, I'll have it done shortly. Would it be worth also addressing #58 in fixing the power settings? Could we add another default value to the class (not unlike addressing the bandwidth options) as to what behaviour the user wants? |
|
I think maybe the correct fix seems to move: To the high level wrapper. We can then add a parameter: 'power_source' that would be |
|
again, I have a philosophy that I want this to work for you the contributor, and for now, this isn't a blocker for me. If it is a blocker for you, then we can work to address it. |
|
I think the refactor should just happen elsewhere. It will also require careful consideration. If it does depend on the device class, we can add secondary conditions to the if statements |
Ported fix for USB power from ps5000a to ps4000a.
Enabled Bandwidth Filtering for PicoScope 4444