fix: disable channel range select when in running state

[Dishebh]

Fixes #2099

Changes

• disabled channel range select dropdown for CH1 and CH2 when in running state under Oscolloscope

Screenshots / Recordings

Checklist:

• No hard coding: I have used resources from strings.xml, dimens.xml and colors.xml without hard coding any value.
• No end of file edits: No modifications done at end of resource files strings.xml, dimens.xml or colors.xml.
• Code reformatting: I have reformatted code and fixed indentation in every file included in this pull request.
• No extra space: My code does not contain any extra lines or extra spaces than the ones that are necessary.

Summary by Sourcery

Fix the issue where the channel range select dropdown was active during the running state of the oscilloscope by disabling it, and refactor dataEntries to use a more consistent type.

Bug Fixes:

• Disable the channel range select dropdown for CH1 and CH2 when the oscilloscope is in the running state to prevent unintended changes.

Enhancements:

• Refactor dataEntries from ArrayList<ArrayList> to List<List> for improved type consistency.

[sourcery-ai]

Reviewer's Guide by Sourcery

This pull request implements a fix to disable the channel range select dropdowns for CH1 and CH2 when the oscilloscope is in the running state. The changes primarily affect the OscilloscopeActivity class, introducing new functionality to manage the enabled state of the spinners based on the oscilloscope's running state.

File-Level Changes

Change	Details	Files
Introduce new UI elements and a method to control their enabled state	Add private fields for spinnerRangeCh1 and spinnerRangeCh2 Initialize the spinner fields in onCreate method Create a new method setChannelRangeEnabledState to enable/disable the spinners Call setChannelRangeEnabledState in onPrepareOptionsMenu	app/src/main/java/io/pslab/activity/OscilloscopeActivity.java
Update the run/stop functionality to manage spinner states	Modify onOptionsItemSelected to call setChannelRangeEnabledState when run/stop state changes Enable spinners when stopping the oscilloscope Disable spinners when starting the oscilloscope	app/src/main/java/io/pslab/activity/OscilloscopeActivity.java
Refactor data structure types for better type safety	Change ArrayList<ArrayList> to List<List> for dataEntries Update references to dataEntries to use the new List<List> type	app/src/main/java/io/pslab/activity/OscilloscopeActivity.java

---

Tips

• Trigger a new Sourcery review by commenting @sourcery-ai review on 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.

[sourcery-ai]

Hey @Dishebh - I've reviewed your changes - here's some feedback:

Overall Comments:

• Consider initializing the spinners (spinnerRangeCh1 and spinnerRangeCh2) once in the onCreate() method and storing them as class members, rather than finding them by ID every time setChannelRangeEnabledState() is called. This would be slightly more efficient.

Here's what I looked at during the review

• 🟡 General issues: 2 issues found
• 🟢 Security: all looks good
• 🟢 Testing: all looks good
• 🟢 Complexity: all looks good
• 🟢 Documentation: all looks good

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.}

[github-actions]

Build successful. APKs to test: https://github.com/fossasia/pslab-android/actions/runs/10966746843/artifacts/1960336218

[Dishebh]

cc @marcnause @AsCress

[Dishebh]

hey @marcnause @AsCress , sorry for tagging you again, but if you get a chance to look at this PR, and find any issues, please let me know. Thanks!

[marcnause]

@sourcery-ai review

[marcnause]

The changes have the described effect. I am just not sure anymore if disabling channel range select is the solution of if it only hides a bug that should be fixed. Please let me investigate and come back to this PR once I have made up my mind.

[AsCress]

Thanks for your work on this issue !
My view is that we could possibly have a better way of solving this instead of just disabling the range select when the Oscilloscope is running.
We have a piece of code which sets the axes range whenever a sample is captured from the Oscilloscope. How about not updating the range everytime, and just setting it once using the range select spinners ?
Maybe we can work on these lines and solve this issue in a better way @Dishebh.
What do you think of this @marcnause ?

[marcnause]

@AsCress I checked the OscilloscopeActivity and I guess line 1185 is the piece of code you are referring to, right? I removed lines 1185 and 1186 locally and I am finally able to set the range as expected. Do you think there may be any negative effects of this change?

One additional problem I noticed: If CH1 and CH2 are selected, both are scaled according to the range set for CH1. The range for CH2 is ignored. But maybe we should handle this problem in an additional issue.

[marcnause]

I think there will be a problem if the lines are removed and a user switches to FFT mode and back. I think the whole issue is more complicated than I thought when I wrote my first comment in #2099. I think it would probably make things easier if all variables and methods that are connected to range selection is moved to an extra class. I will try to do some refactoring tonight if I find the time.

[AsCress]

@marcnause Hmm, I see. I think there may also be a problem with the Auto-Scale. Do inform me if you need any help with this (currently I am working a lot on the ESP and the Logic Analyzer, so didn't get time to push any changes to this PR or any other 😢).

[marcnause]

I have added a new pull request in which I try to fix the underlying problem: #2552

[AsCress]

@Dishebh Since this is now superseeded by #2552, are you ok with closing this PR ?

[Dishebh]

Hey, sure. Let me close this, if Marc's PR already fixes the issue. Thanks, @marcnause, for fixing this, and sorry I couldn't do it 😅

[marcnause]

@Dishebh Thank you very much for taking on this problem. Your contribution was the first step towards solving this problem, without it the bug ticket would probably still be open!