Reimplement alarm muting

[ethanjli]

This PR resolves #372 and fixes #420 by completely reimplementing alarm muting functionality.

Main functionality

This PR makes the following changes to the communication protocol:

• Makes the firmware, backend, and frontend cancel any active alarm mute upon detection of a connection loss. By making all peers have the same behavior (actively setting AlarmMute.active to false) during a network partition, it is possible to achieve consistency in the value of AlarmMute.active during a network partition (cf. the CAP theorem in distributed systems). In practice we will sacrifice availability by disabling alarm mute initiation functionality in the frontend during a network partition. We restore full consistency for AlarmMute after the network partition ends through the way we use AlarmMute.seq_num (described in the next bullet point).
• Adds a seq_num (sequence number) field to AlarmMute and AlarmMuteRequest which provides idempotency on AlarmMuteRequests and provides a way to reject inconsistencies between AlarmMute and AlarmMuteRequest after a connection is lost and restored, by always using the firmware's copy of AlarmMute as the source of truth when there is a conflict with AlarmMuteRequest. Refer to the main description of #372 for more details.
• Adds a AlarmMuteSource to AlarmMute and AlarmMuteRequest, and more values of LogEventCode, so that the firmware can generate event log messages recording alarm mute activation and cancellation events which indicate why an activation or cancellation occurred (e.g. user action on a GUI button, user action on a membrane button, automatic timeout, loss of connection)
• Moves BackendConnections from frontend_pb.proto to mcu_pb.proto, and makes the backend send BackendConnections to the firmware, so that the firmware can cancel any active alarm mute when the backend cannot receive any messages from the firmware but the firmware can receive messages from the backend, or when the frontend cannot receive any messages from the backend but the frontend can send messages to the backend and the backend can send messages to the firmware. This will also be needed for the firmware to prevent the user from activating an alarm mute via membrane button while the backend is unable to receive any data from the frontend.
• Changes the remaining field of AlarmMute/AlarmMuteRequest to be a uint64 with units of ms, to be consistent with all other timestamps and durations in the communication protocol.

React component decomposition & redraw optimization

This PR makes the following related improvements to the frontend's modules/app/EventAlerts.tsx, much of which already had to be reimplemented for the alarm mute cancellation behavior:

• Moves messy code which was in useStates and a useEffect into selectors. This makes their logic easier to understand (and easier to test), because selectors are just pure functions without side-effects or mutable state. And all the mutable state already lived in the redux store - those useStates were trying to duplicate the data which was already in the store, which introduces the risk of inconsistency. We should look out for other places we can do the same thing in the frontend - this is a good first step for refactoring.
• Decomposes the EventAlert topbar component into an Alarms parent component in modules/alarms/Alarms.tsx integrating a bunch of new child components in modules/alarms/Alarms.tsx, modules/alarms/AlarmMuting.tsx, and modules/logs/EventLog.tsx so that:
  • Instead of returning a giant block of JSX for the topbar, the parent component returns a group of child components for the topbar. This makes the EventAlert component more readable, it allows each child to be understood in isolation, it allows each child to be tested in isolation, and it also enables the improvements described in the next bullet points.
  • The parent component does not use any useSelector statements, as I found that was the reason everything was getting redrawn whenever any single selector had an updated value (this was probably the root cause of #420). Furthermore, each child component uses the minimal possible number of useSelector statements and only returns things which depend on those useSelector statements; anything else should be moved to a different component, so that it isn't forced into an extraneous redraw whenever a selector updates.
  • Any useEffects which aren't directly needed to redraw components (i.e. which only modify state or dispatch redux actions) are moved into their own component which returns nothing, so that update of the dependencies of any useEffects won't also trigger an extraneous redraw of rendered JSX.
  • Any child components which need to access or modify shared local state should do so via getters and setters passed by the parent component into their props. This allows us to share state between components without having to add a bunch of stuff to the redux store, and it also establishes a very clear responsibility for the parent component: the parent just takes responsibility for owning the local state shared among child components and for grouping together the child components.
  • When we need to conditionally render some JSX, we can use if (condition) { return <></>; } return component;-style statements in a child component to make the conditional logic more explicit and less indented. This reduces the need for {condition && (component)} snippets inside JSX returns, which aren't supposed to be rendered as false or 0 - except I've rarely had that happen to me in the past in this codebase, for reasons I don't understand.
• Cleans up component and variable names by removing all internal uses of the word "Alert" and replacing it with "Alarm". After this PR, the file name and the exported component name still need to be renamed.
• Removes style classes which are no longer used.
• Removes the AlertToast component, which was deprecated and already wasn't imported anywhere else in the frontend.

@rohanpurohit I think the kind of decomposition done in EventAlerts.tsx is what we should do as the first goal for improving the quality of any components with giant return blocks of JSX (for understandability, maintainability, and testability), as well as any components causing extraneous redraws (for performance/responsiveness). Based on the outcomes of the refactor in this PR, I'm confident that thoughtful decomposition will solve most or all of our problems with extraneous redraws.

When we need to reuse the same component in many different places, and each place needs to duplicate the same set of useStates, we could consider moving the duplicated local states into the redux store to make the code simpler and reduce boilerplate; this may also reduce the number of places we have to pass around getter and setter callbacks, as those can probably just be replaced with selectors and action dispatchers.

The refactor in this PR doesn't use containers as I understand them (i.e. react-redux's connect), because the original code just relied on a fixed set of useSelectors and didn't need to be parameterized on interchangeable selectors (i.e. none of the React components were generic code). However, react-redux's useSelector behaves pretty similarly to connect (except for reference vs. shallow equality and some other details). When we have a react component which should work with different selectors and simultaneously need to reuse the same selector-component combination in many different places, we should use containers; otherwise, we could just combine it with different selectors using the approach I've taken in EventAlerts.tsx by passing in a selector and use it with useSelector (I believe this is what ValueSelectorDisplay does). However, there's an important caveat from react-redux's hooks documentation: "unlike connect(), useSelector() does not prevent the component from re-rendering due to its parent re-rendering, even if the component's props did not change." - this means that wherever we use useSelector in a child component, we have to make sure that the parent component, the grandparent component, the great-grandparent, etc., all avoid useSelector. (unrelatedly, that documentation provides another useful tip about React.memo() which might be very useful for removing extraneous redraws in the event log table).

etc

This PR makes the following related improvements to the backend and firmware:

• The simulator backend's implementation of the alarm mute service is now a very close port (almost one-to-one) of the firmware's implementation of this service.
• Instead of turning on the Nucleo board's LED whenever there is an active alarm, now the firmware only turns on the LED whenever there is an active alarm and the alarms are not muted.
• The simulator backend now generates log messages indicating when there is an active unmuted alarm and when there are no longer active unmuted alarms. This corresponds to the Nucleo board's behavior.
• The firmware code comment documentation in Pufferfish/Application/States.h for what to do when adding a new message type has now been updated and made more helpful and complete.
• Commented-out testing/troubleshooting/debugging code has been added to the backend's ventserver.protocols.backend.states for cycling every 10 seconds between sending events to the MCU and not sending events. This provides a way to make the MCU think that the backend has disconnected, without causing a restart of the MCU which would happen if we unplugged/replugged the Nucleo board's USB cable.
• Whitespace in the backend's ventserver.simulation.power_management has been improved (for some reason the existing pylint checks don't catch these).

This PR adds the following functionalities to the frontend:

• Defines a new EphemeralLogEventAction redux action and updates the event log reducer to handle it, so that the frontend can generate arbitrary ephemeral log events (which get wiped once the backend connects and sends a NextLogEvents message). This is needed so that the frontend can generate an ephemeral log event recording that it cancelled an alarm mute after the backend disconnects; either the backend or firmware will generate a non-ephemeral log event, depending on whether the backend also detected loss of the connection to the frontend, so the ephemeral event will be overwritten with a non-ephemeral event (providing consistency during/after a connection problem).
• Shows seconds in addition to hours and minutes in the Event Log timestamps, to provide more detailed information about when events occurred. This exposes one or more bugs in the clock synchronization protocol (#424).

This PR fixes the following unrelated bugs in the frontend:

• Records the actual lastBackendConnectionTime in store/connection/reducers.ts, instead of always saving it as the value set during app initialization (which happens to be null).

This PR also makes the following readability/maintainability improvements:

• Improves the organization of protobuf-related files by adding code comments to all files corresponding to different sections of the protobuf files, and reordering firmware/frontend lines to match the order of type definitions in the protobuf files.

[ethanjli]

For records-keeping:

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

@rohanpurohit Since this is a reimplementation of alarm muting functionality and a major refactor of EventAlerts.tsx, it'd be helpful to me if you could test those things with a fresh mind, as some things may have slipped past me. It might also be worth taking a look at how modules/app/EventAlerts.tsx has changed into modules/alarms/Alarms.tsx, modules/alarms/muting/AlarmMuting.tsx, and modules/logs/EventLog.tsx, as a companion to reading the "React component decomposition & redraw optimization" section of the main PR description which outlines what I did to stop the wasteful redraws. Everything else in the main PR description was stuff we already discussed in today's meeting, so you can skip everything else in the PR description.

[rohanpurohit]

For records-keeping:

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

@rohanpurohit Since this is a reimplementation of alarm muting functionality and a major refactor of EventAlerts.tsx, it'd be helpful to me if you could test those things with a fresh mind, as some things may have slipped past me. It might also be worth taking a look at how modules/app/EventAlerts.tsx has changed into modules/alarms/Alarms.tsx, modules/alarms/muting/AlarmMuting.tsx, and modules/logs/EventLog.tsx, as a companion to reading the "React component decomposition & redraw optimization" section of the main PR description which outlines what I did to stop the wasteful redraws. Everything else in the main PR description was stuff we already discussed in today's meeting, so you can skip everything else in the PR description.

Just did light to moderate testing on https://github.com/pez-globo/pufferfish-software/pull/428/commits/a5c60b5ed830c65d37b79ece9be7919683ddf713, and everything works as intended, there are no regressions from the refactor!

• Looked at the refactor of EventAlerts.tsx and yes! I see the benefits of everything you mentioned in the PR description and the advantages of the approach on redraws by decomposing really large components into smaller children. It's easier to maintain, less redraws, and easier to understand!
• The Events log redraws are fixed now! and everything looks really good!
• Continuing on that point, yes there are many places where we loosely use useSelectors and useEffects without taking into account the shared state or potential redraw of the parent component, I should definitely be careful about that in the future.
• But I still feel in some components where we dispatch actions frequently (like ValueClickers) we may not be able to avoid redraws even with really careful decomposition and or state management, maybe there we need to be effective about how we dispatch actions, for example: with a single button click like submit or something similar, in which case it redraws only once!

I was going to approve, but I saw that you are still making changes so did not do that!

[ethanjli]

• But I still feel in some components where we dispatch actions frequently (like ValueClickers) we may not be able to avoid redraws even with really careful decomposition and or state management, maybe there we need to be effective about how we dispatch actions, for example: with a single button click like submit or something similar, in which case it redraws only once!

In general, the best we can do is that the things which change get redrawn while everything else does not. If the action dispatches only happen under less-frequent conditions, I think we can tolerate extra redraws there if we can't refactor them out, we'll have to see on a case-by-case basis - the goal of eliminating extra redraws throughout the app is so that we have reasonable responsiveness and performance in the places where reducing the rate of redraws would degrade usability

I was going to approve, but I saw that you are still making changes so did not do that!

~I'm done with changes now!~ Correction, I need to run yarn lint:fix.