fix: reset atem upon connection #307
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## release51 #307 +/- ##
=============================================
- Coverage 40.77% 40.75% -0.03%
=============================================
Files 105 105
Lines 10001 10009 +8
Branches 2463 2464 +1
=============================================
+ Hits 4078 4079 +1
- Misses 5387 5394 +7
Partials 536 536 ☔ View full report in Codecov by Sentry. |
| }, | ||
|
|
||
| resetToState: async (state: any) => { | ||
| await this._stateHandler.setCurrentState(state) | ||
| await this._stateHandler.clearFutureStates() | ||
| this.emit('resetResolver') |
There was a problem hiding this comment.
This isn't something to fix now, but doesn't calling this.emit('resetResolver') result in clearing the state of every device and rebuilding it all from the timeline again?
That feels like a rather heavy operation to do whenever a device manages to connect, especially as the states returned by the resolver will not be different to before.
Perhaps something in each service could cache the states as given by the resolver, so that resetState and resetToState can reuse those instead?
There was a problem hiding this comment.
yeah, i think we have enough data in the statehandler class to just handle rebuilding the device states and diffs in there without going to the conductor. Definitely one to consider.
| resetState: () => Promise<void> | ||
|
|
||
| /** Reset the tracked device state to "state" and notify the conductor to reset the resolver */ | ||
| resetToState: (state: any) => Promise<void> // todo - types? |
There was a problem hiding this comment.
Unresolved TODO
I agree it would be good to have the type of the state to be known here. It will probably be a bit sprawling, but should be easy enough to do as a generic parameter to this interface
There was a problem hiding this comment.
My initial worry was that we already use a lot of generics and it does become a bit of a sprawl but in this case it's only a few places that need to be aware actually.
This is a bug fix.
Expected behaviour
The atem integration will initiate correctly and enforce its state.
Observed behaviour
The atem integration does a state diff before it has connected to the actual device. This returns no commands. The State Handler than transitions to this state and expects it to be current.
Once the device is connected, a
resetResolveris called. This does not reset the state itself causing it to diff against an incorrect current state and will again return no commandsProposed fix
Add a method to update the current state in the State Handler on the context. In addition, chane the
resetStatecontext method to change the current state toundefined