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

Maker: Reliable Board Reset #13252

Merged
merged 2 commits into from Feb 15, 2017

Conversation

Projects
None yet
2 participants
@islemaster
Member

islemaster commented Feb 15, 2017

Perform a stateless board reset and full re-initialization every time you run the app.

Fixed Issues

At least these; there are probably more.

  1. LED colors persist across resets
    This is actually a failure to reset library state on the host PC, not failure to reset the board. Demonstrated by this app on production.

    1. Run the app. LED lights up white.
    2. Click the button. LED turns red.
    3. Reset the app.
    4. Run the app again.
      Expected: The LED lights up white.
      Actual: The LED lights up red.
  2. When running maker in multiple tabs it just stops working after a while.
    Once a particular tab loses its connection it cannot get it back.

    1. Open a maker app on production (the first example works well)
    2. Run the app, see the board work, reset the app.
    3. Open a maker app (can be the same one) in a second tab.
    4. Run the app, see the board work, reset the app.
    5. Switch back to the first tab.
    6. Run the app.
      Expected: The board should work just like it did in step 2.
      Actual: App fails to connect to the board.

Before

Both of these problems were caused by a complex stateful initialization process, that was in part driven by how long it used to take for the board to connect. It went like this:

  • On first run (per page load):
    1. Create an interpreter (this is part of applab)
    2. Initialize the serial port, playground-io/Firmata and johnny-five libraries.
    3. Initialize all of the board components.
    4. Install components on the interpreter.
  • On reset:
    1. Destroy the interpreter (this is part of applab)
    2. Reset board components one at a time.
  • On 2nd and subsequent runs
    1. Create an interpreter
    2. Install cached components on the interpreter

The LED color issue arises from the fact that their component reset doesn't work properly (and I suspect this is true for many components). The multiple tabs issue happens because the first tab doesn't know that the second tab has invalidated its cached port/board/components.

The new approach

Stateless is the best! We now treat every run like the first run, and do the full initialization process (which is super fast now).

On reset we call Firmata's reset() function, which sends 0xFF system reset to the board (see the Firmata protocol), basically rebooting the firmware so we know everything's back in a good state. Then we completely throw away our board/component objects.

To do this we're fighting with some statefulness within the Firmata, Johnny-Five and Playground-IO libraries. This was partially resolved in #13172 which stopped Johnny-Five from using its static board cache when setting up components by explicitly injecting a board object into each component. For this change we've also modified playground-io again to make it possible to construct it more than once on a page. I believe this is a legitimate bug in playground-io and have also submitted this change upstream so hopefully we can switch back if/when it's accepted.

What's next

There's a ton of cleanup that can happen in BoardController now - a lot of the initialization code is jumping through hoops to handle this now-defunct board/component caching nicely - but the stability change of this benefit is so great that I'd like to ship this minimal change and make the rest a refactor.

@islemaster islemaster requested a review from Hamms Feb 15, 2017

Show outdated Hide outdated apps/package.json Outdated
this.board_.register.concat(touchSensors).forEach(BoardController.resetComponent);
this.board_.io.reset();
this.board_ = null;
this.prewiredComponents = null;

This comment has been minimized.

@Hamms

Hamms Feb 15, 2017

Contributor

this looks a lot more like a destroy method now than a reset; where do we expect these values to be reinitialized?

@Hamms

Hamms Feb 15, 2017

Contributor

this looks a lot more like a destroy method now than a reset; where do we expect these values to be reinitialized?

This comment has been minimized.

@islemaster

islemaster Feb 15, 2017

Member

Good point, maybe I'll rename this to destroy during followup refactoring.

BoardController's connectAndInitialize() is already set up to handle both the first run and subsequent run cases, by following a lazy-create pattern; now that we are destroying the board and components here, it will always follow it's first-run path and reinitialize them every time.

@islemaster

islemaster Feb 15, 2017

Member

Good point, maybe I'll rename this to destroy during followup refactoring.

BoardController's connectAndInitialize() is already set up to handle both the first run and subsequent run cases, by following a lazy-create pattern; now that we are destroying the board and components here, it will always follow it's first-run path and reinitialize them every time.

@Hamms

Hamms approved these changes Feb 15, 2017

@islemaster islemaster merged commit 1f89c4a into staging Feb 15, 2017

2 checks passed

ci/circleci Your tests passed on CircleCI!
Details
hound No violations found. Woof!

@islemaster islemaster deleted the maker-true-reset branch Feb 15, 2017

@islemaster islemaster changed the title from Maker: Reliable Board Reset (Part One) to Maker: Reliable Board Reset Feb 17, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment