pez-globo / pufferfish-software

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

Play audio in frontend for all active alarms #337

Closed rohanpurohit closed 3 years ago

rohanpurohit commented 3 years ago

Currently:

Bugs:

Can be reproduced on develop as well. seems to be a state issue or useEffect

rohanpurohit commented 3 years ago

Few observations that still exist and are mostly related to how the alarmMuteStatus store and then the local state is behaving:

  1. start ventilation -> any alarm is triggered -> pause ventilation -> start ventilation again-> if an alarm is still active, it is muted on start, maybe that behavior is not preferred?
  2. alarm sound is playing, and a page is changed (for instance to set_alarms page) and then we try to mute the alarm, need to toggle it twice to get it working as expected.
  3. item 2 can also be observed sometimes when ventilation is paused and restarted

other than that, muting the alarm seems to be fine when we don't navigate from the dashboard. probably is a very small fix to solve the above-mentioned issues but needs debugging.

  1. This project is licensed under Apache License v2.0 for any software, and Solderpad Hardware License v2.1 for any hardware - do you agree that your contributions to this project will be under these licenses, too? yes.
  2. Were any of these contributions also part of work you did for an employer or a client? no.
  3. Does this work include, or is it based on, any third-party work which you did not create? no.
ethanjli commented 3 years ago

I'm unable to reproduce item 1 when I test with the simulator. Patient alarms should indeed be cancelled in quickstart (this is taken care of by AlarmsService, and new alarms get triggered when ventilation starts again if the condition is still there to trigger the alarm), though we'll want to have the low battery alarm still active in quickstart. I'm also unable to reproduce item 2. Can you file a new issue for each of these things, and we can follow up on them separately from this PR? I suspect they are independent problems from the audio playing: it sounds like item 2 related to the implementation of the alarm muting (which will definitely be changed), while item 1 is something else.

rohanpurohit commented 3 years ago

I'm unable to reproduce item 1 when I test with the simulator. Patient alarms should indeed be cancelled in quickstart (this is taken care of by AlarmsService, and new alarms get triggered when ventilation starts again if the condition is still there to trigger the alarm), though we'll want to have the low battery alarm still active in quickstart. I'm also unable to reproduce item 2. Can you file a new issue for each of these things, and we can follow up on them separately from this PR? I suspect they are independent problems from the audio playing: it sounds like item 2 related to the implementation of the alarm muting (which will definitely be changed), while item 1 is something else.

maybe I dint describe the first item properly

will create issues and we can take it from there.