Skip to content

Fix #1823 xy plot working fine in oscilloscope #1828

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Jun 26, 2019
Merged

Fix #1823 xy plot working fine in oscilloscope #1828

merged 5 commits into from
Jun 26, 2019

Conversation

@neel1998
Copy link
Member

@neel1998 neel1998 commented Jun 24, 2019 •
edited
Loading

Fixes #1823

Changes: Fixed xyPlot code in oscilloscope

Screenshot/s for the changes:
20190626_145832

Checklist: [Please tick following check boxes with [x] if the respective task is completed]

  • I have used resources from strings.xml, dimens.xml and colors.xml without hard-coding them
  • No modifications done at the end of resource files strings.xml, dimens.xml or colors.xml
  • I have reformatted code in every file included in this PR [CTRL+ALT+L]
  • My code does not contain any extra lines or extra spaces
  • I have requested reviews from other members

APK for testing:

osc_xy_plot3.zip

CloudyPadmal
CloudyPadmal requested changes Jun 24, 2019
View reviewed changes
app/src/main/java/io/pslab/activity/OscilloscopeActivity.java Outdated

if (scienceLab.isConnected() && viewIsClicked && isXYPlotSelected) {
if (scienceLab.isConnected() && isXYPlotSelected) {
Log.d("xy plot", "called");
Copy link
Collaborator

@CloudyPadmal CloudyPadmal Jun 24, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's remove this part.

Copy link
Member Author

@neel1998 neel1998 Jun 24, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I added for debugging

app/src/main/java/io/pslab/activity/OscilloscopeActivity.java
HashMap<String, double[]> data;
if ("CH2".equals(xyPlotXAxisChannel) || "CH2".equals(xyPlotYAxisChannel)) {
analogInput = params[0];
data = scienceLab.captureTwo(1000, 10, analogInput, false);
Copy link
Collaborator

@CloudyPadmal CloudyPadmal Jun 24, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideal method would be this. With current implementation it fetches one trace and then fetch the second trace after sometime; not simultaneous. CaptureTwo fetches them simultaneously.

Copy link
Member Author

@neel1998 neel1998 Jun 24, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. will make the change

@CloudyPadmal CloudyPadmal added the Status: Review Required Requested reviews from peers and maintainers label Jun 24, 2019
@neel1998
Copy link
Member Author

neel1998 commented Jun 25, 2019 •
edited
Loading

@CloudyPadmal the captureTwo()function is giving weird output. Also i couldn't understand how captureTwo() can be used to get data for two random channels say CH3 and MIC. I also checked the code of new desktop App , there also fetchTrace() function is called sequentially. For the delay you mentioned earlier i have removed the sleep() call between two fetchTrace() calls

@CloudyPadmal
Copy link
Collaborator

CloudyPadmal commented Jun 26, 2019

Also i couldn't understand how captureTwo() can be used to get data for two random channels say CH3 and MIC.

I looked through the code for you because you couldn't understand. If you compare the changes you made with the previous code, it's already there anyways.

  • CaptureTwo fetches CH1 and CH2. If you want random channel selection, it was in an else statement in previous code using CaptureFour where it fetches all four channels CH1, CH2, CH3, MIC. From that you can filter out the channels you want.
  • May be it's a good thing if you can include JavaDocs to those methods as you read and understand.

If the channel readings are giving weird outputs, may be increase timeout/sleep periods in methods in ScienceLab.java class.

I also checked the code of new desktop App , there also fetchTrace() function is called sequentially.

I don't remember how many times I've said it, take only Python Desktop App as reference for code implementations (Not for UI). I'll open an issue in Electron repo for the case.

@neel1998
Copy link
Member Author

neel1998 commented Jun 26, 2019

@CloudyPadmal thanks for your help and time. Will keep your advice in mind regarding desktop app. Sorry for inconvenience caused

@neel1998
Copy link
Member Author

neel1998 commented Jun 26, 2019

@CloudyPadmal have updated the apk and gif. Have a look. Thanks

@cweitat cweitat merged commit 8e02c26 into fossasia:development Jun 26, 2019
neel1998 pushed a commit to neel1998/pslab-android that referenced this pull request Jul 30, 2019
@cweitat
Merge pull request fossasia#1828 from neel1998/oscilloscope_xy_plot
6935884 
Fix fossasia#1823 xy plot working fine in oscilloscope
@neel1998 neel1998 deleted the oscilloscope_xy_plot branch August 27, 2019 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cweitat cweitat cweitat approved these changes

@CloudyPadmal CloudyPadmal CloudyPadmal approved these changes

Assignees

@neel1998 neel1998

Labels

Status: Review Required Requested reviews from peers and maintainers

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

XY plot feature doesn't work properly in oscilloscope

3 participants

@neel1998 @CloudyPadmal @cweitat