search

pez-globo / pufferfish-software

All software for the Pufferfish ventilator.
Apache License 2.0
0 stars 1 forks source link

Frontend Start/Pause Ventilation button behavior when MCU is lost #361

Closed ethanjli closed 3 years ago

ethanjli commented 3 years ago

When MCU is lost, allow the frontend to tell the backend that it should tell the MCU to Start/Pause Ventilation once the MCU is back up. In other words, the Start/Pause Ventilation button shouldn't be automatically changed to disabled. However, whenever parameters.current.ventilating != parameters.request.ventilating, the Start/Pause ventilation button should say "Connecting..."

This issue was split off from https://github.com/pez-globo/pufferfish-software/pull/344#issuecomment-841357911

ethanjli commented 3 years ago

@rohanpurohit How much of this issue is covered by #370? I see you implemented "Connecting..." button text, but I'm not sure about the "When MCU is lost, allow the frontend to tell the backend that it should tell the MCU to Start/Pause Ventilation once the MCU is back up" part.

rohanpurohit commented 3 years ago

but I'm not sure about the "When MCU is lost, allow the frontend to tell the backend that it should tell the MCU to Start/Pause Ventilation once the MCU is back up" part.

Ya, this is not implemented yet, we are kind of blocked by having a selector for the connection right, that would make it easy?

ethanjli commented 3 years ago

That makes sense, so this issue is pending completion of #371

ethanjli commented 3 years ago

Sequence of events (strongly needed):

  1. Ventilation is active
  2. Firmware/backend disconnects, and firmware is still ventilating
  3. User does nothing
  4. The button still says "Pause Ventilation" and is enabled
  5. Firmware/backend reconnects, and firmware is still ventilating

Sequence of events (desirable):

  1. Ventilation is active
  2. Firmware disconnects
  3. User presses "Pause Ventilation" button
  4. The button changes to say ~"Connecting..."~"Pausing..." and is disabled
  5. Firmware reconnects, and it doesn't start ventilation again

Sequence of events (optional/undesirable but tolerable):

  1. Ventilation is inactive
  2. Firmware disconnects
  3. User presses "Starts Ventilation" button
  4. The button changes to say "Connecting..." and is disabled
  5. Firmware reconnects, and it starts ventilation
ethanjli commented 3 years ago

Sequence of events (preferred, and what we already have):

  1. Ventilation is inactive
  2. User presses "Start Ventilation" button
  3. The button changes to say "Starting..." and is disabled
  4. The dashboard opens, and the button changes to say "Pause Ventilation" and be enabled

Sequence of events (preferred):

  1. Ventilation is inactive
  2. Firmware disconnects
  3. The button changes to say "Connecting..." and is disabled
  4. Firmware reconnects
  5. The button changes back to say "Start Ventilation" and is enabled

Sequence of events (preferred):

  1. Ventilation is inactive
  2. User presses "Start Ventilation" button
  3. The button changes to say "Starting..." and is disabled
  4. Firmware disconnects
  5. The button doesn't change
  6. Firmware reconnects
  7. The dashboard opens, etc.
rohanpurohit commented 3 years ago

Decision Table for Start Ventilation Button

<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:x="urn:schemas-microsoft-com:office:excel" xmlns="http://www.w3.org/TR/REC-html40">

Ventilation | Firrmware Connected | Request | Response | Button Label | Button Disabled -- | -- | -- | -- | -- | -- FALSE | TRUE | TRUE | TRUE | Pause Ventilation | NO FALSE | TRUE | TRUE | FALSE | Starting… | YES FALSE | TRUE | FALSE | FALSE | Start Ventilation | NO FALSE | FALSE | TRUE | - | Starting… | YES FALSE | FALSE | FALSE | - | Start Ventilation | NO FALSE | FALSE | - | - | Connecting… | YES TRUE | TRUE | TRUE | TRUE | Pause Ventilation | NO TRUE | TRUE | FALSE | TRUE | Pausing… | YES TRUE | TRUE | FALSE | FALSE | Start Ventilation | NO TRUE | FALSE | TRUE | - | Pause Ventilation | NO TRUE | FALSE | FALSE | - | Pausing… | YES

@ethanjli Let me know if this is what you had in mind, and if there are any changes/improvements, I think maybe I covered them all? let me know if I missed any case and/or if something is not clear from my end.

ethanjli commented 3 years ago

Some comments:

rohanpurohit commented 3 years ago

Some comments:

  • I assume the first column ("Ventilation") means "response.ventilating"

Yes the first column is whether it's ventilating

  • I believe the second column should actually be "Backend and Firmware Both Connected"

Yep! will change

  • Does "-" mean "the message is null in the store"? I will proceed assuming this is what you mean; if so, let's explicitly write "null" or "undefined" instead of "-"

it's not null it's mostly cases where parameters.request.ventilating != parameters.current.ventilating so maybe undefined or unchanged?

if this is the case, then the fourth column ("Response") is actually redundant, and doesn't make sense for Ventilation to be FALSE while Response is TRUE (or vice versa) - so the 1st, 9th rows don't make sense to me. Or did you mean something else for the first column or the fourth column

So I added the 4th column to include the cases where we send the request and don't receive a response back as the firmware is disconnected, and also cases where we have sent a request and until we get it back that is when parameters.request.ventilating != parameters.current.ventilating, we need to display say for example starting.. in the 2nd row.

  • The 5th row (false/false/false) should be the same as the 6th row (false/false/undefined): if the firmware is disconnected while we're in standby mode, we should wait until the firmware is connected before we can request to start ventilation. This is the second sequence of events in Frontend behavior when MCU is lost #361 (comment). yep missed that! that's right. If we make this change to the 5th row, then the 4th row should be unreachable; for simplicity, let's make that be the same as the 6th row too.

Yep will do that.

ethanjli commented 3 years ago

So here's my mental model of what your table (with the fix to the 5th row and the change to the 4th row) is equivalent to:

firmware & backend connected frontend store's response.ventilating frontend store's request.ventilating button label button disabled
true true true Pause Ventilation false
true true false Pausing... true
true false true Starting... true
true false false Start Ventilation false
false true true Pause Ventilation false
false true false Pausing... true
false false/undefined/null true Connecting... true
false false/undefined/null false Connecting... true

Does this table match your mental model of what the behavior of the button should be? I believe we can refactor it to the following table, which should be exactly equivalent - let me know if you can find any errors:

frontend store's response.ventilating frontend store's request.ventilating firmware & backend connected button label button disabled
true true true or false Pause Ventilation false
true false true or false Pausing... true
false/undefined/null true true Starting... true
false/undefined/null false true Start Ventilation false
false/undefined/null true or false false Connecting... true

When the firmware or backend is disconnected, the only time the frontend can correctly make any assumptions about whether the firmware is running ventilation is when request.ventilating and response.ventilating have both been false for a while before the disconnection (i.e. there's no request.ventilating = true which might have been in transit when the disconnection occurred), in which case we can assume that the firmware is still in standby mode. Otherwise, the frontend has no way to know whether the firmware is ventilating.

rohanpurohit commented 3 years ago

yes @ethanjli that table matches my mental model, and It looks good, thank you for making the changes and simplifying it!

rohanpurohit commented 3 years ago

Final Decision table: after discussion in #391 comment

<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:x="urn:schemas-microsoft-com:office:excel" xmlns="http://www.w3.org/TR/REC-html40">

frontend store's response.ventilating | frontend store's request.ventilating | firmware & backend connected | Button Label | Button Disabled -- | -- | -- | -- | -- true | true | true or false | Pause Ventilation | false true | false | true or false | Pausing… | true false/undefined/null | true | true or false | Starting… | true false/undefined/null | false | true | Start Ventilation | false false/undefined/null | false | false | Connecting… | true