drivers/dstate: improve ALARM status safeguards, documentation and handling

docs/new-drivers.txt

@@ -218,6 +218,10 @@ formatting string to those expectations.  In this case, you would use
 	char *fmt = "Mega-Zapper %d";
 	dstate_setinfo_dynamic("ups.model", fmt, "%d", rating);
 
+Please note that `ups.alarm` should no longer be manually set, but rather
+the appropriate alarm functions should be used instead. For more details,
+see below in the `UPS alarms` section.
+
 Setting flags
 ~~~~~~~~~~~~~
 
@@ -261,9 +265,10 @@ Possible values for status_set:
 	BOOST   -- UPS is boosting incoming voltage
 	FSD     -- Forced Shutdown (restricted use, see the note below)
 
-Internally, an `ALARM` value would be added (typically as first in the list)
-if the `ups.alarm` is currently not empty. For more details, see below in
-`alarm_set()` description.
+`ALARM` should no longer be raised through the UPS status.
+An `ALARM` value will be added internally (typically as first in the list)
+if an alarm was set and committed through the appropriate alarm functions.
+For more details, see below in the `UPS alarms` section.
 
 [NOTE]
 ======

drivers/dstate.c

@@ -50,7 +50,8 @@
 	static OVERLAPPED	connect_overlapped;
 	static char	*pipename = NULL;
 #endif	/* WIN32 */
-	static int	stale = 1, alarm_active = 0, alarm_status = 0, ignorelb = 0;
+	static int	stale = 1, alarm_active = 0, alarm_status = 0, ignorelb = 0,
+				alarm_status_set = 0, alarm_legacy_mode = 0;
 	static char	status_buf[ST_MAX_VALUE_LEN], alarm_buf[ST_MAX_VALUE_LEN],
 			buzzmode_buf[ST_MAX_VALUE_LEN];
 	static conn_t	*connhead = NULL;
@@ -1801,6 +1802,7 @@ static int status_set_callback(char *tgt, size_t tgtsize, const char *token)
 		 */
 		upsdebugx(2, "%s: (almost) ignoring ALARM set as a status", __func__);
 		if (!alarm_status && !alarm_active && strlen(alarm_buf) == 0) {
+			upsdebugx(2, "%s: Assume sloppy driver coding that ignored alarm methods and did not set an alarm message: set an injected N/A value", __func__);
 			alarm_init();	/* no-op currently, but better be proper about it */
 			alarm_set("[N/A]");
 		}
@@ -1870,14 +1872,33 @@ void status_commit(void)
 	 */
 
 	if (!alarm_active && alarm_status && !strcmp(alarm_buf, "[N/A]")) {
-		upsdebugx(2, "%s: Assume sloppy driver coding that ignored alarm methods and used status_set(\"ALARM\") instead: commit the injected N/A ups.alarm value", __func__);
+		upsdebugx(2, "%s: Assume sloppy driver coding that ignored alarm methods and used status_set(\"ALARM\") instead: commit the alarm state entrance", __func__);
 		alarm_commit();
+		alarm_legacy_mode = 1;
+	}
+
+	if (alarm_legacy_mode && alarm_status_set && !alarm_status) {
+		/* The ALARM status token has just disappeared, probably the driver
+		 * removed it without calling the appropriate functions. We also
+		 * need to exit the alarm state here, otherwise the ALARM status
+		 * token will get added back to the UPS status variable later.
+		 *
+		 * We only want this to happen for legacy drivers which set
+		 * ALARM without using the appropriate functions, otherwise
+		 * the ALARM state should be decoupled from the UPS status.
+		 */
+		upsdebugx(2, "%s: Assume sloppy driver coding that ignored alarm methods and just removed previously active ALARM from status: commit the alarm state exit", __func__);
+		alarm_init();
+		alarm_commit();
+		alarm_legacy_mode = 0;
 	}
 
 	if (alarm_active) {
 		dstate_setinfo("ups.status", "ALARM %s", status_buf);
+		alarm_status_set = 1;
 	} else {
 		dstate_setinfo("ups.status", "%s", status_buf);
+		alarm_status_set = 0;
 	}
 }