Closed ethanjli closed 3 years ago
Running the tests on my local machine:
backend running ventserver.application:
start ventilation
from the quickstart page did not start the ventilation even after multiple triggers.start ventilation
-> refresh page -> goes to the dashboard -> press pause -> goes back to quickstart but button still says pause ventilation
(not sure if I was supposed to change anything for application)
backend running ventserver.simulator:
spo2_too_high
the alarm was registered in active_alarms modal and event logs too, there was no sound like when the backend is stopped and there is a sound for software connection lost
fio2_too_low
alarm was active when alarm limits were changed to trigger an alarm for spo2_too_low
which flooded the events log modal with 20-30 entries for one trigger? and similarly, the events log modal kept flooding for subsequent triggers.Running the tests on the ventilator unit:
backend running ventserver.application:
ventilation stopped
etc correctly.spo2 sensor
therefore it was simulating, changing spo2_alarm_limits triggered alarms for both spo2_too_low and spo2_too_high backend running ventserver.simulator:
This is extremely helpful, thanks for these details!
Backend running ventserver.application on your local machine: I think I'll need to understand your test conditions more thoroughly in order to understand what's happening (for example, was a Nucleo board connected up together with all the sensors). Let's discuss this in today's call. If you're able to screenshare what you see when you run this test during the meeting, that'll be very helpful.
Backend running ventserver.simulator on your local machine:
Backend running ventserver.application on the ventilator:
Backend running ventserver.simulator on the ventilator:
Things to do in future PRs or issues:
Issues to try to reproduce and document so that Ethan can reproduce:
Small fixes for this PR:
@rohanpurohit I've solved some issues with flooding of the event log. When you have a chance, can you play around with b128343 to see if the issues you previously saw with flooding the events log have been fixed, and/or if there are other issues with the events log, e.g. when the backend is restarted?
Can you also see whether/how you can reproduce the "stopping the backend gives 'software connection lost' along with the alarm, but no entry in the events log modal or active alarms modal" behavior you had observed? I've changed some things in the frontend which might interact with the code related to this behavior, but nothing which I'd expect to have fixed this issue.
Running the tests on my local machine:
backend running ventserver.application:
- press
start ventilation
-> refresh page -> goes to the dashboard -> press pause -> goes back to quickstart but button still sayspause ventilation
(Test env- local machine without an STM32 connected to it)
Can you also see whether/how you can reproduce the "stopping the backend gives 'software connection lost' along with the alarm, but no entry in the events log modal or active alarms modal" behavior you had observed? I've changed some things in the frontend which might interact with the code related to this behavior, but nothing which I'd expect to have fixed this issue.
I have not been able to reproduce this behavior since, maybe we can rule it out for now? I'm not sure under what circumstances I saw that behavior
@rohanpurohit I've solved some issues with flooding of the event log. When you have a chance, can you play around with b128343 to see if the issues you previously saw with flooding the events log have been fixed, and/or if there are other issues with the events log, e.g. when the backend is restarted?
The flooding issue seems to have improved, but I'd say it's still there?. unfortunately, I don't have access to a ventilator unit or STM32 connected to sensors therefore can't test it in application mode, although I had seen similar flooding in the previous test on application too.
To reproduce the behavior on backend running ventserver.simulator:
Ok, that looks good. There should be flooding on alarms when measurements are very near alarm limits, because I haven't fixed that (progress on that will be tracked in #322). If I fully fixed the other flooding issues, there shouldn't be flooding on other events (e.g. parameter changes or alarm limits changes).
@rohanpurohit In this PR I've defined more MessageTypes, and once I merged develop in, some of your MessageTypes parse tests, which assume that no message types have values above 7, have started failing. We'll need those tests to work even after I define more message types, so maybe the unit tests should have their own definition of the MessageTypes
enum. How do you want to proceed?
Because this PR has been reasonably well-tested (though testing on the final refactoring and fixes has been much lighter) and is part of the v0.7 milestone, I'm going to merge this into develop as-is, and as prototype-grade code. For records-keeping:
This mega-PR contains a lot of changes everywhere (I'll try to split things up into smaller PRs in the future!). The most significant one is that the alarms and event logging and list synchronization code from the backend have now also been ported to the firmware, so we have alarms working in fully-integrated system with real hardware.
Firmware:
Pufferfish::Application::States
to include message types for the event logventserver.protocols.application.lists.SendSynchronizer
toPufferfish::Application::Protocols::Lists::ListSender
. This implementation differs from the backend's implementation in that this implementation discards each element whose receipt has been acknowledged by the backend (via theExpectedLogEvent
message), because the STM32 doesn't have enough RAM to keep all events in memory.ventserver.simulation.logs
toPufferfish::Application::LogEvents::IncrementalEventsSender
. This is a small wrapper around the list sender which ensures that event IDs increment for each event, and also that event timestamps are correct.ventserver.simulation.alarm_limits
toPufferfish::Driver::BreathingCircuit::AlarmLimitsService
. This handlesAlarmLimitsRequest
s and updatesAlarmLimits
accordingly.ventserver.simulation.alarms
toPufferfish::Driver::BreathingCircuit::AlarmsService
. This checks sensor measurements against alarm limits and activates (and deactivates) alarm events accordingly. We will need to modify the implementations of these things in the future to have a timeout for how long a value is out of range before the alarm activates or deactivates - essentially, "debouncing" the alarms. This will be needed because when we adjust the FiO2 and flow settings, the firmware will automatically adjust the alarm limits; and initially they will be out-of-range while FiO2 and flow rate adjust to the new settings values, but during this initial period we don't want to trigger an alarm.Pufferfish::Driver::BreathingCircuit::ParametersService
. Everything should still work correctly, but I might have introduced regressions. It would be great if people could test various input values parameters to see if anything breaks, e.g. if invalid inputs are allowed or valid inputs are prohibited/ignored by the firmware.Util::RingBuffer
to work not just onuint8_t
s, but also on other types. I've also added new functionality for RingBuffers not used with interrupts (i.e. every use besides in UARTBuffers), namely look-ahead peek and size reporting. These have not been thoroughly tested, so additional testing and bug-hunting of creating events (the event log uses those RingBuffer functionalities) in different temporal patterns would be appreciated.erase
method toPufferfish::Util::Vector
. The erase method is used byUtil::OrderedMap
, described next.Pufferfish::Util::OrderedMap
as a basic associative key-value container (backed by aPufferfish::Util::Vector
). I'd appreciate review of the function interface and feedback on the safety and usability of the method interface provided by this class. I also haven't tested this thoroughly, so additional testing and bug-hunting of activating and deactivating alarm conditions (which updates anOrderedMap
) in various temporal patterns would be appreciated.Backend:
ventserver.application
no longer initializes all values of all state segments to 0 on program startup. Instead, it initializes them toNone
so that they can be set by the frontend and/or firmware (and in the future initial defaults will be set but they will all be overwritten by the filesystem)ventserver.protocols.application.lists.SendSynchronizer
now accepts resetting theExpectedLogEvent.id
field to 0. This is because if the frontend restarts after a crash, it will resetExpectedLogEvent.id
to 0 and it should load all logged events.ventserver.protocols.backend.ReceiveFilter
now has some member objects which handle for receiving log events from the MCU (which is a prerequisite for #310) and sending new events to the frontend; the MCU's event IDs and relative timestamps are first translated into the backend's event IDs and Unix epoch timestamps (which fixes #311, but relies on time remapping only being done on timestamps in LogEvents, not on timestamps in any other messages), and some bugs are fixed related to flooding of event logs.Frontend:
ExpectedLogEvent
andNextLogEvents
so that receipt of new log events will immediately trigger an increase toExpectedLogEvent.id
, rather than the Logs Page triggering this increase through a dispatch. This fixes a bug where if I didn't open the Event Log or Active Alarms modal at least once in the frontend, then new alarms wouldn't trigger the topbar and alarm border until I did open that modal. However, it does expose some issue related to the frontend switching from the dashboard back to the quick start screen and then back to the dashboard when the backend is restarted, under certain conditions. I think this is a separate issue related to state initialization; it may or may not be resolved together with #312.Protobuf schemas:
uint32
touint64
, so that we can store Unix epoch timestamps as integers with millisecond precisionI would appreciate testing of the software on this PR - please report all problems you see, and please test it in various configurations (with different combinations of computers, hardware,
ventserver.application
, andventserver.simulator
); we will fix smaller problems related to changes from this PR in this PR before merging, and we will create Github issues for larger problems or unrelated problems to address in future PRs. We will merge this in as prototype-quality code, but we should identify any regressions or breakage caused here and either fix it here or prioritize it for the next PR.