search

phetsims / balloons-and-static-electricity

"Balloons and Static Electricity" is an educational simulation in HTML5, by PhET Interactive Simulations.
http://phet.colorado.edu/en/simulation/balloons-and-static-electricity
GNU General Public License v3.0
6 stars 10 forks source link

Instrument for phet-io #204

Closed samreid closed 7 years ago

samreid commented 7 years ago

In order to support https://github.com/phetsims/a11y-research/issues/2 and to have another sim instrumented for phet-io.

samreid commented 7 years ago

@ariel-phet and @jessegreenberg I'm getting close to a point where I can start instrumenting Balloons and Static Electricity for PhET-iO, but I wanted to ask if we should use this as an opportunity to get @jessegreenberg involved with PhET-iO instrumentation? I worked with @jbphet yesterday to start instrumentation of Gravity Force Lab, and it seems the instructional documentation is mostly working and the process is mostly clear.

Balloons and Static Electricity will also face the question of how to deal with PhET-iO for the new accessibility features. For instance, there are 9 new files in balloons-and-static-electricity/js/accessibility that may need attention during PhET-iO instrumentation.

So before I start I thought I'd ask @ariel-phet if we should get @jessegreenberg involved in this instrumentation, and, if so, to what extent? (i.e. taking the lead on it like @jbphet is for Gravity Force Lab vs consulting with me regarding the accessibility files).

ariel-phet commented 7 years ago

@samreid I think it will be pretty critical to get @jessegreenberg involved since it is seeming likely the iO instrumentation has potential benefits on the a11y side, particularly as it relates to sonification and the ability to data collection for interviews and such. So the more familiar JG is with the architecture and procedure the better.

I think ideally it would be good for you and @jessegreenberg to collaborate on this effort at first, and then if the a11y related questions get answered reasonably early to move into more of a consulting role and have JG finish it off.

@jessegreenberg I am not sure how tight your schedule is on the a11y side of things but it seems it would be good to move ahead with this effort at high priority because of the recording goal. Let me know if that impression is incorrect.

samreid commented 7 years ago

@ariel-phet that sounds like a nice plan. @jessegreenberg let me know your thoughts.

jessegreenberg commented 7 years ago

This sounds great! I would love to get learn more about the iO instrumentation process, and given its potential for accessibility it seems very useful.

Now seems like a pretty good time to begin this. The latest prototypes are out and waiting for interview results and a few design considerations. High priority seems appropriate to me, but @emily-phet should also be aware of this.

samreid commented 7 years ago

Notes from yesterday's collaboration:

samreid commented 7 years ago

@emily-phet I wanted to apprise you of the status on this. @jessegreenberg and I got Balloons and Static Electricity partially instrumented, and you can observe the positions of the balloons and some of the controls in the state playback (charges and some controls, and the wall do not play back properly). Once we have completed more of this checklist in https://github.com/phetsims/balloons-and-static-electricity/issues/204#issue-189248033, the sim will be closer to being able to be recorded for playback, though we will still need to write a new playback parser to load states instead of events.

emily-phet commented 7 years ago

@samreid Thanks for the update! I'm excited to hear this work is progressing. Do you have a sense of how many hours will be required to complete full instrumentation of this sim?

samreid commented 7 years ago

I'm unfamiliar with the new accessibility code for the simulation. Without it, I would have guessed a total of 16 hours to instrument the simulation. We have already spent around 4 hours or so. But the new accessibility code or how to instrument it for PhET-iO is somewhat of an unknown.

emily-phet commented 7 years ago

Thanks, good to know.

samreid commented 7 years ago

@ariel-phet pointed out that @emily-phet has another use case coming up for Jan 15 which could benefit from having this sim instrumented as well.

@ariel-phet suggested that @jbphet and @jessegreenberg pair together to complete the instrumentation of Balloons and Static Electricity (without instrumenting the accessibility subdirectory yet).

Once this sim is instrumented, I can work with @zepumph to enable playbacks based on state in https://github.com/phetsims/phet-io/issues/921

samreid commented 7 years ago

@jbphet and @samreid will pick this up for today, and @ariel-phet will discuss with @emily-phet regarding priorities for @jessegreenberg.

samreid commented 7 years ago

@jbphet will pick up from the checklist in https://github.com/phetsims/balloons-and-static-electricity/issues/204#issue-189248033 and consult with @samreid as questions come up.

samreid commented 7 years ago

Balloons and static electricity is not affected by the tandem/observablearray issues we discussed earlier. @jbphet you are cleared for takeoff.

ariel-phet commented 7 years ago

Spoke with @emily-phet -- currently it is not 100% clear what ETS will be after, but she will try to extract that information.

However, our GT collaborators for sonification will also be interested in this sim so much like BAA it will be useful to have instrumented.

To begin the goal will just be "normal" instrumentation, and not yet worrying about instrumenting a11y features.

jbphet commented 7 years ago

I did a fair amount of work on this today. For reference, I'm on the step Replace types with tandemized equivalents, and I've worked through the types:

 TandemButtonListener
 TandemSimpleDragHandler
 TandemCircle
 TandemImage
 TandemNode
 TandemPath

The types

TandemRectangle
TandemText

are up next.

Also, I noticed that this sim uses the scenery node Line, so that should be done soon also.

emily-phet commented 7 years ago

I emailed ETS about their specific needs. They said they would discuss and get back to me.

jbphet commented 7 years ago

@samreid mentioned to me that one of the first usages of this sim will be to test accessibility and record the state information. In order to make sure we can support this, the state wrapper should be tested particularly thoroughly.

jbphet commented 7 years ago

There is a node called PlayAreaNode that I instrumented much of, and when I went to test it I realized it was only visible when the dev query parameter was set. I'm not sure whether this should be instrumented at all. It's not quite done yet, but I'd like to make sure it's worth the effort to continue on it.

@samreid and @jessegreenberg - any input on this?

samreid commented 7 years ago

It seems it should not be instrumented for phet-io because it is a development tool rather than an end-user feature.

jbphet commented 7 years ago

@jessegreenberg - please review commit https://github.com/phetsims/balloons-and-static-electricity/commit/54ade832f7b4a0431b119d5983cf58112410181c, since I ended up changing slightly when the a11y code is executed and you should make sure that this is okay.

jessegreenberg commented 7 years ago

@jbphet I will trade you! Can you please review https://github.com/phetsims/balloons-and-static-electricity/commit/4b1449971da5e597c5543fbf63b9c7fe43ae97e2? I had to make a big change to KeyboardHelpDialog for #232 and it is no longer a Dialog. I updated the tandems accordingly, but may not have gotten it correct.

jessegreenberg commented 7 years ago

The change looks good to me @jbphet, the description is still updating at the correct time.

jbphet commented 7 years ago

@jessegreenberg - on https://github.com/phetsims/balloons-and-static-electricity/commit/4b1449971da5e597c5543fbf63b9c7fe43ae97e2 - BASEKeyboardHelpContent.js is not currently common code, so the tandem should be required and not part of the options. This is the convention for instrumented sims. In this commit, it looks like it is a parameter to the constructor that then goes into the default options hash, which is kind of a mix of the option/required approaches.

If this is intended to become common code at some point, then having it as an option may make sense. In that case, it shouldn't be a parameter of the constructor, just an option with a default value. You can search for 'tandem:' in the sun repo for examples. In this case, you should probably document that it is optional due to this file being bound for common code and add a TODO.

jessegreenberg commented 7 years ago

Thanks @jbphet, that makes sense. This is not something that is going to move to common code, so it there is no need to have the tandem in options. Now that Dialog is no longer used, the tandem is not used anywhere else throughout the file so I removed it entirely. Perhaps when we work together on instrumenting the types related to accessibility, we can revisit this file.

emily-phet commented 7 years ago

@jbphet - can you send me a link to the instrumented sim, to pass along to collaborators, once you're done?

emily-phet commented 7 years ago

@jbphet Can you estimate when this will be ready to share with collaborators?

jbphet commented 7 years ago

There is one more minor a11y feature to instrument, then I'd like to do some testing using all the different wrappers. Assuming that all works out reasonably well, we can have a version ready to share with collaborators tomorrow, Jan 24 2017.

I am still a little foggy on exactly what the collaborators plan to do with this, so I'd like to create a dev version and have someone test it before producing a release. @emily-phet - who currently has the knowledge to do that testing?

emily-phet commented 7 years ago

@jbphet That's great! I met with ETS last week, and they're aware of the limitations intrinsic to tracking certain interactions specific to assistive devices. A basic implementation of PhET-iO is fine for their purposes. The only added feature that they hoped for was the ability to track focus changes. I believe you and @jessegreenberg were looking into this. If that's available, wonderful, but if not we should go ahead and send it without this week, as they're hoping to start working with the PhET-iO features soon. The folks with ETS who have worked with PhET-iO before are also involved with this collaboration, so they're aware of what to expect from a basic implementation.

samreid commented 7 years ago

@emily-phet - who currently has the knowledge to do that testing?

@phet-steele has designed a test procedure for testing PhET-iO simulations and their instrumentation, and has applied it for several simulations.

jbphet commented 7 years ago

I'll try to get an RC together today (this assumes I don't run into any major issues) and set up a test task. @emily-phet - when are you hoping to be able to share this with collaborators? The QA team will likely need to know in order to prioritize their testing.

emily-phet commented 7 years ago

@jbphet Well, we were originally shooting for a mid-January delivery, so now I think we're at "as soon as possible". If it will not be ready before the end of this week, please let me know so I can give our collaborators a heads up.

jbphet commented 7 years ago

An RC version has been published for this initial milestone, and the associated test task can be found here: https://github.com/phetsims/tasks/issues/770.

jbphet commented 7 years ago

This has been done and has gone through the QA process (see https://github.com/phetsims/tasks/issues/777). We've been using the phet-io versions for sonification wrappers for a while now and it's working fine. Closing.