pslab-android
pslab-android copied to clipboard
refactor range select in oscilloscope
Fixes #2099
Changes
- refactoring of scale handling: moved scale values to extra class
- only set scale when required, not on every new data frame
Checklist:
- [ ] No hard coding: I have used resources from
strings.xml,dimens.xmlandcolors.xmlwithout hard coding any value. - [x] No end of file edits: No modifications done at end of resource files
strings.xml,dimens.xmlorcolors.xml. - [x] Code reformatting: I have reformatted code and fixed indentation in every file included in this pull request.
- [x] No extra space: My code does not contain any extra lines or extra spaces than the ones that are necessary.
Summary by Sourcery
Refactor the oscilloscope's range selection by encapsulating scale values in a new class and optimizing scale updates to occur only when necessary. This improves code structure and performance.
Enhancements:
- Refactor the oscilloscope's scale handling by moving scale values to a dedicated class, improving code organization and maintainability.
- Introduce a mechanism to set axis scales only when necessary, reducing unnecessary updates and improving performance.
- Replace anonymous inner classes with lambda expressions for cleaner and more concise code.
Build successful. APKs to test: https://github.com/fossasia/pslab-android/actions/runs/11670470293/artifacts/2142682113
@AsCress: In https://github.com/fossasia/pslab-android/pull/2551 you mentioned, that the auto-scale-feature may have problems too. I think it only changes the scale for the left Y-axis. Is that correct?
I have added two comments which contain TODO and your name. I am not sure if what I have done there is correct and it would be great if you could take an extra close look there.
Static code analysis fails due to high complexity in one of the Fragments.
Reviewer's Guide by Sourcery
This pull request refactors the range select functionality in the oscilloscope feature. The main changes involve introducing a new OscilloscopeAxisScale class to encapsulate axis scale values and their management, and updating related classes to use this new abstraction. This refactoring aims to improve code organization, reduce duplication, and enhance maintainability.
File-Level Changes
| Change | Details | Files |
|---|---|---|
| Introduction of OscilloscopeAxisScale class |
|
app/src/main/java/io/pslab/others/OscilloscopeAxisScale.java |
| Refactoring OscilloscopeActivity to use OscilloscopeAxisScale |
|
app/src/main/java/io/pslab/activity/OscilloscopeActivity.java |
| Updating fragments to use OscilloscopeAxisScale |
|
app/src/main/java/io/pslab/fragment/ChannelParametersFragment.javaapp/src/main/java/io/pslab/fragment/DataAnalysisFragment.javaapp/src/main/java/io/pslab/fragment/TimebaseTriggerFragment.javaapp/src/main/java/io/pslab/fragment/XYPlotFragment.java |
| Code cleanup and modernization |
|
app/src/main/java/io/pslab/activity/OscilloscopeActivity.javaapp/src/main/java/io/pslab/fragment/ChannelParametersFragment.javaapp/src/main/java/io/pslab/fragment/DataAnalysisFragment.javaapp/src/main/java/io/pslab/fragment/TimebaseTriggerFragment.javaapp/src/main/java/io/pslab/fragment/XYPlotFragment.java |
Tips
- Trigger a new Sourcery review by commenting
@sourcery-ai reviewon the pull request. - Continue your discussion with Sourcery by replying directly to review comments.
- You can change your review settings at any time by accessing your dashboard:
- Enable or disable the Sourcery-generated pull request summary or reviewer's guide;
- Change the review language;
- You can always contact us if you have any questions or feedback.
This PR breaks Fourier Transform in Data Analysis screen of the Oscilloscope. Therefore I will change it to WIP.
Fourier Transform works again!
@AsCress, could you please check if the oscilloscope still works as expected for you?
@marcnause This is a fantastic PR ! The idea to clean the code by refactoring it into a different file is an excellent one !
I've fixed some general issues regarding the mV (milli volt) scale and removed the TODOs after reviewing them.
We're good to go with this PR.
This only issue left to tackle is the rightYAxisScale; changing it has no affect on the graph.
We'd have to fix this by plotting the channel 2 output according to the Right Y Axis, however, that I think should be done in a different PR :))
@AsCress thank you for your review! Unfortunately, you don't seem to have write access yet and I am not allowed to review your changes since I started the PR. I guess we need either @CloudyPadmal or @mariobehling to approve. 👀
Note: Static code analysis fails due to high complexity in a method I touched, but complexity has been high before that already.
@marcnause Yes that's correct :))
I noticed that the automatic measurements show strange values when the trigger is moved to a value out of the range of the current signal. (As discussed in the call today.)
@marcnause Can you confirm if this issue still exists? Strangely, I wasn't able to reproduce this on my device today 😕.
The issue still exists, I was able to reproduce it with the APK from this PR and 2 PSLab boards.
@marcnause I see. This issue seems to be more device-specific as I can reproduce this on some devices that I have and not on others. Also, this issue doesn't arise due to this PR, but due to something else. In any case, we can solve it here and I'll push to this PR very soon solving this issue.
I have created a new issue (https://github.com/fossasia/pslab-android/issues/2567) and will merge the code in this PR.