Skip to content
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

Measure Activity for Sugarizer #708

Closed
wants to merge 26 commits into from

Conversation

abhishektanwar
Copy link
Contributor

Developed the Measure Activity in sugar for Sugarizer.
measure_1

measure_2

@abhishektanwar
Copy link
Contributor Author

#709

@quozl
Copy link

quozl commented Mar 17, 2020

57b113c commit message is a bit brief. You could say what you used, like p5 sound.

I see you added exec = sugar-activity-web. Will you be porting this to Sugar or does it work already?

@abhishektanwar
Copy link
Contributor Author

abhishektanwar commented Mar 17, 2020

@quozl @llaske I am developing this activity for Sugarizer. Sugar has the measure activity and I took the inspiration from https://help.sugarlabs.org/measure.html .A few days ago I asked in sugar-devel to make this activity for sugarizer.
I have used the p5.js ,p5.sound and p5.dom libraries in this activity.
There is more work that needs to be done like plotting waveform for different octaves,play a sound and show how waveform is varying at different amplitudes.

@llaske
Copy link
Owner

llaske commented Mar 17, 2020

Good start. Nice.

You should also adapt the UI to Sugarizer. Specifically, settings should be in a palette, not directly on the string. Plus the buttons in the toolbar should be selected/unselected instead of hidden.

@abhishektanwar
Copy link
Contributor Author

@llaske Thanks for the feedback.
By settings do you mean the sliders at the bottom of the page ?
Also can the x-axis scale be shown on the graph or do I need to place it somewhere else?If I have to place it somewhere else,where should it be placed?
Working on suggestions.

@llaske
Copy link
Owner

llaske commented Mar 17, 2020

Yes I mean the sliders.
Regarding the x-axis scale, may be you could place it at the bottom of the graph like in Measure activity.

@abhishektanwar
Copy link
Contributor Author

Working on the suggestions.

@abhishektanwar
Copy link
Contributor Author

@llaske I am finding difficulties in moving the sliders to the toolbar palette. As the current sliders have been made using p5.js and these sliders are directly linked to the waveforms if I am making any changes to these, everything breaks. Could you provide me some documentation for sliders in sugarizer . I have tried to implement the working of sliders from different activities but it didn't work.

@llaske
Copy link
Owner

llaske commented Mar 23, 2020

@abhishektanwar sliders in Sugarizer is just HTML input with the type range. You could find a very basic sample here. It's used for example in Speak or Game of Life activities.
Regarding the current sliders related to p5.js code, may be you could just hide them.

@abhishektanwar
Copy link
Contributor Author

@llaske thanks for the help it worked.
I have fixed all the requested changes. Please review.
Activity is complete now.

@abhishektanwar
Copy link
Contributor Author

@ashish0910 @llaske removed console logs. Please review.

@llaske
Copy link
Owner

llaske commented Mar 28, 2020

Mmmm. The activity does nothing on my side.
At start the screen is totally black:

image

When the graph button is clicked the activity seems to listen but nothing happen:

image

Plus will be nice to improve icons/slide bar proportions/alignement in palette:

image

@abhishektanwar
Copy link
Contributor Author

@llaske @ashish0910 Waveform is plotted from the values of sliders (amplitude and frequency). Initially, the values of amplitude and frequency slider is 0(zero). As the sliders are moved, the corresponding waveform is plotted.

  • Fixed alignment of icons and sliders in palette.
    Please review

measure_3a

@llaske
Copy link
Owner

llaske commented Mar 29, 2020

So do not start with a value of zero for amplitude and frequency and display the waveform at launch.
Note sure to understand why sliders values change when switching graph?
Plus, will be nice to save values (and current graph selection) in the datastore so the user could reopen the activity to recover its context.

@abhishektanwar
Copy link
Contributor Author

thanks for the feedback. working on it.

@abhishektanwar
Copy link
Contributor Author

@llaske @ashish0910

  • added datastore functionality
  • fixed initial zero value of sliders
  • on initializing an old instance of the activity , activity starts from previous slider settings and last waveform(time or frequency)
  • saves current graph when activity is stopped

please review

@abhishektanwar
Copy link
Contributor Author

I suggest to start the activity paused, in this case only (if on Chome and if file://).

I think to maintain the working of activity similar to for all the browsers , activity can be started with pause state (the button which is being used to pause the graph) and then the user can start the activity by clicking on play button or we can proceed as you suggested earlier by tweaking the pause button for chrome browser.

@llaske
Copy link
Owner

llaske commented Apr 23, 2020

I'm not agree. The activity should start automatically.
Paused at startup is just a work around.

@abhishektanwar
Copy link
Contributor Author

I'm not agree. The activity should start automatically.
Paused at startup is just a work around.

Okay then activity will start normally for all browsers . But for chrome ,activity will start with paused state . User will have to click play button.
Please let me know if this is fine.

@llaske
Copy link
Owner

llaske commented Apr 23, 2020

But for chrome ,activity will start with paused state

For Chrome, only when run from file://

@abhishektanwar abhishektanwar requested a review from llaske April 24, 2020 03:49
@llaske
Copy link
Owner

llaske commented Apr 24, 2020

The activity no longer work on http:// :

Capture du 2020-04-24 21-31-07

@llaske llaske removed their request for review April 24, 2020 19:32
@abhishektanwar
Copy link
Contributor Author

can you layout the steps you followed to run this activity from the beginning . It worked fine on my side @llaske

@llaske
Copy link
Owner

llaske commented Apr 24, 2020

Just cleaning the browser cache. Then launch a new instance of Measure.

@abhishektanwar
Copy link
Contributor Author

@llaske there was one error . Fixed it . Please review again.

@abhishektanwar abhishektanwar requested a review from llaske April 26, 2020 23:40
@llaske
Copy link
Owner

llaske commented Apr 27, 2020

In http mode it works but the title "Sound time base" is displayed under graph:

Capture d’écran du 2020-04-27 18-06-20
Capture d’écran du 2020-04-27 18-05-56

In file mode the graph is not drawn correctly:

Capture d’écran du 2020-04-27 18-07-39

Try to do more testing before sending your PR.

@abhishektanwar
Copy link
Contributor Author

@llaske it is working fine on my side in file mode .
If you are using some other method to testing other than I am using please let me know .
When in chrome and file mode , I am showing the empty graph ,user clicks on play button which also accounts for the policy of chrome that a user gesture is required for accessing microphone.

Peek 2020-04-28 00-40

@llaske
Copy link
Owner

llaske commented Apr 27, 2020

Do you clear your browser cache?

@llaske llaske removed their request for review April 27, 2020 20:25
@abhishektanwar
Copy link
Contributor Author

@llaske Yes, I have cleared all the cached files in the chrome browser.
measure

@llaske
Copy link
Owner

llaske commented Apr 28, 2020

Sound like it happens randomly. When it happens, there is a message file protocol error in the console:

Capture d’écran 2020-04-28 à 21 15 58

Another issue, when switching graph in pause mode, a double graph appears:

erreree

@abhishektanwar
Copy link
Contributor Author

Sound like it happens randomly. When it happens, there is a message file protocol error in the console:

What can we do with this?What do you recommend?

Another issue, when switching graph in pause mode, a double graph appears:

Yes it can happen because according to previous discussions activity was designed to switch between time and frequency graphs only when the graph is in play mode (not in pause state).

@llaske
Copy link
Owner

llaske commented Apr 28, 2020

What can we do with this?What do you recommend?
Try to explore in which condition this can happen and fix it.

Yes it can happen
Okay but it should be solve. We can't let it like that.

@abhishektanwar
Copy link
Contributor Author

Sound like it happens randomly. When it happens, there is a message file protocol error in the console:

I have tweaked the activity for chrome a bit . Please review it again . Works fine on mine side(tested after clearing cache)

Another issue, when switching graph in pause mode, a double graph appears:

Fixed

@abhishektanwar abhishektanwar requested a review from llaske April 28, 2020 20:48
@llaske
Copy link
Owner

llaske commented May 1, 2020

Still few issues:

  • The graph should start automatically in Chrome when not in file mode. My request was to pause the activity at start in Chrome AND in file mode
  • When the activity is reopened, the double background issue still appears:

Peek 01-05-2020 22-08

  • I've got several times the random error described before. Following is some screen capture when it happens. Did you have the issue sometimes on your side?

Capture d’écran du 2020-05-01 21-56-48
Capture d’écran du 2020-05-01 21-48-07

  • Remove commented lines and //change comment

@llaske llaske removed their request for review May 1, 2020 20:09
@llaske
Copy link
Owner

llaske commented Jan 4, 2023

Replaced by #709

@llaske llaske closed this Jan 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants