Refactor wireless status monitoring to use direct sysfs file descriptor polling instead of udev hid/usb subsystem events

spa/plugins/alsa/alsa-udev.c

@@ -47,9 +47,13 @@ enum wireless_status {
 	WIRELESS_STATUS_DISCONNECTED,
 };
 
+/* Forward declaration for struct impl */
+struct impl;
+
 /* This represents an ALSA card.
  * One card can have up to 1 PCM and 1 Compress-Offload device. */
 struct card {
+	struct impl *impl;
 	unsigned int card_nr;
 	struct udev_device *udev_device;
 	unsigned int unavailable:1;
@@ -70,6 +74,8 @@ struct card {
 	uint32_t compress_offload_device_id;
 
 	char *wireless_status_path;
+	int wireless_status_fd;
+	struct spa_source wireless_status_source;
 };
 
 static uint32_t calc_pcm_device_id(struct card *card)
@@ -108,6 +114,11 @@ struct impl {
 	int use_ucm;
 };
 
+/* Forward declarations */
+static void process_card(struct impl *this, enum action action, struct card *card);
+static void stop_wireless_status_monitor(struct impl *this, struct card *card);
+static void on_wireless_status_change(struct spa_source *source);
+
 static int impl_udev_open(struct impl *this)
 {
 	if (this->udev == NULL) {
@@ -135,7 +146,9 @@ static struct card *add_card(struct impl *this, unsigned int card_nr, struct ude
 
 	card = &this->cards[this->n_cards++];
 	spa_zero(*card);
+	card->impl = this;
 	card->card_nr = card_nr;
+	card->wireless_status_fd = -1;
 	udev_device_ref(udev_device);
 	card->udev_device = udev_device;
 
@@ -154,8 +167,8 @@ static struct card *find_card(struct impl *this, unsigned int card_nr)
 
 static void remove_card(struct impl *this, struct card *card)
 {
+	stop_wireless_status_monitor(this, card);
 	udev_device_unref(card->udev_device);
-	free(card->wireless_status_path);
 	this->n_cards--;
 	if (card != &this->cards[this->n_cards])
 		*card = this->cards[this->n_cards];
@@ -165,8 +178,8 @@ static void clear_cards(struct impl *this)
 {
         unsigned int i;
 	for (i = 0; i < this->n_cards; i++) {
+		stop_wireless_status_monitor(this, &this->cards[i]);
 	        udev_device_unref(this->cards[i].udev_device);
-		free(this->cards[i].wireless_status_path);
 	}
 	this->n_cards = 0;
 }
@@ -276,6 +289,15 @@ static void unescape(const char *src, char *dst)
 	*d = 0;
 }
 
+static enum wireless_status parse_wireless_status(const char *buf)
+{
+	if (spa_streq(buf, "connected"))
+		return WIRELESS_STATUS_CONNECTED;
+	if (spa_streq(buf, "disconnected"))
+		return WIRELESS_STATUS_DISCONNECTED;
+	return WIRELESS_STATUS_UNKNOWN;
+}
+
 static enum wireless_status read_wireless_status(const char *sysfs_path)
 {
 	char buf[16];
@@ -296,12 +318,7 @@ static enum wireless_status read_wireless_status(const char *sysfs_path)
 	if (buf[sz - 1] == '\n')
 		buf[sz - 1] = '\0';
 
-	if (spa_streq(buf, "connected"))
+	return parse_wireless_status(buf);
-		return WIRELESS_STATUS_CONNECTED;
-	if (spa_streq(buf, "disconnected"))
-		return WIRELESS_STATUS_DISCONNECTED;
-
-	return WIRELESS_STATUS_UNKNOWN;
 }
 
 static bool is_usb_interface_dir(const char *name)
@@ -407,21 +424,93 @@ static char *find_wireless_status_path(struct impl *this, const char *card_syspa
 	return NULL;
 }
 
-static void update_wireless_status(struct impl *this, struct card *card)
+static void stop_wireless_status_monitor(struct impl *this, struct card *card)
 {
+	if (card->wireless_status_fd >= 0) {
+		spa_loop_remove_source(this->main_loop, &card->wireless_status_source);
+		close(card->wireless_status_fd);
+		card->wireless_status_fd = -1;
+	}
+	if (card->wireless_status_path) {
+		free(card->wireless_status_path);
+		card->wireless_status_path = NULL;
+	}
+}
+
+static int start_wireless_status_monitor(struct impl *this, struct card *card, const char *path)
+{
+	char buf[16];
+	ssize_t sz;
+	int fd;
+
+	fd = open(path, O_RDONLY | O_CLOEXEC);
+	if (fd < 0) {
+		spa_log_debug(this->log, "card %u: failed to open wireless_status: %s",
+			      card->card_nr, spa_strerror(-errno));
+		return -errno;
+	}
+
+	sz = read(fd, buf, sizeof(buf) - 1);
+	if (sz <= 0) {
+		spa_log_debug(this->log, "card %u: failed to read wireless_status: %s",
+			      card->card_nr, spa_strerror(sz < 0 ? -errno : -EIO));
+		close(fd);
+		return sz < 0 ? -errno : -EIO;
+	}
+
+	lseek(fd, 0, SEEK_SET);
+
+	card->wireless_status_path = strdup(path);
+	if (!card->wireless_status_path) {
+		close(fd);
+		return -ENOMEM;
+	}
+
+	card->wireless_status_fd = fd;
+	card->wireless_status_source.func = on_wireless_status_change;
+	card->wireless_status_source.data = card;
+	card->wireless_status_source.fd = fd;
+	card->wireless_status_source.mask = SPA_IO_ERR;
+
+	spa_loop_add_source(this->main_loop, &card->wireless_status_source);
+
+	spa_log_info(this->log, "card %u: monitoring wireless_status at %s",
+		     card->card_nr, path);
+
+	return 0;
+}
+
+static void on_wireless_status_change(struct spa_source *source)
+{
+	struct card *card = source->data;
+	struct impl *this = card->impl;
 	enum wireless_status status;
 	bool disconnected;
+	char buf[16];
+	ssize_t sz;
 
-	if (!card->wireless_status_path)
+	lseek(card->wireless_status_fd, 0, SEEK_SET);
-		return;
+	sz = read(card->wireless_status_fd, buf, sizeof(buf) - 1);
-
+	if (sz <= 0) {
-	status = read_wireless_status(card->wireless_status_path);
-
-	if (status == WIRELESS_STATUS_UNKNOWN) {
 		spa_log_info(this->log, "card %u wireless_status unreadable, removing monitor",
 			     card->card_nr);
-		spa_clear_ptr(card->wireless_status_path, free);
+		stop_wireless_status_monitor(this, card);
 		card->wireless_disconnected = false;
+		process_card(this, ACTION_CHANGE, card);
+		return;
+	}
+
+	buf[sz] = '\0';
+	if (buf[sz - 1] == '\n')
+		buf[sz - 1] = '\0';
+
+	status = parse_wireless_status(buf);
+	if (status == WIRELESS_STATUS_UNKNOWN) {
+		spa_log_info(this->log, "card %u wireless_status unknown, removing monitor",
+			     card->card_nr);
+		stop_wireless_status_monitor(this, card);
+		card->wireless_disconnected = false;
+		process_card(this, ACTION_CHANGE, card);
 		return;
 	}
 
@@ -430,6 +519,7 @@ static void update_wireless_status(struct impl *this, struct card *card)
 		spa_log_info(this->log, "card %u wireless device %s",
 			     card->card_nr, disconnected ? "disconnected" : "connected");
 		card->wireless_disconnected = disconnected;
+		process_card(this, ACTION_CHANGE, card);
 	}
 }
 
@@ -658,17 +748,27 @@ static int emit_added_object_info(struct impl *this, struct card *card)
 
 	snprintf(path, sizeof(path), "hw:%u", card->card_nr);
 
-	if (!card->wireless_status_path) {
+	if (card->wireless_status_fd < 0) {
 		const char *syspath = udev_device_get_syspath(udev_device);
-		if (syspath)
+		char *wireless_path;
-			card->wireless_status_path = find_wireless_status_path(this, syspath);
-	}
 
-	if (card->wireless_status_path) {
+		if (syspath && (wireless_path = find_wireless_status_path(this, syspath))) {
-		enum wireless_status status = read_wireless_status(card->wireless_status_path);
+			enum wireless_status status = read_wireless_status(wireless_path);
-		if (status == WIRELESS_STATUS_DISCONNECTED) {
+
-			card->wireless_disconnected = true;
+			if (status == WIRELESS_STATUS_DISCONNECTED) {
-			return -ENODEV;
+				spa_log_info(this->log, "card %u: wireless device disconnected, not emitting",
+					     card->card_nr);
+				free(wireless_path);
+				card->wireless_disconnected = true;
+				return -ENODEV;
+			}
+
+			if (status == WIRELESS_STATUS_CONNECTED) {
+				if (start_wireless_status_monitor(this, card, wireless_path) < 0)
+					free(wireless_path);
+			} else {
+				free(wireless_path);
+			}
 		}
 	}
 
@@ -917,7 +1017,6 @@ static void process_card(struct impl *this, enum action action, struct card *car
 
 	switch (action) {
 	case ACTION_CHANGE: {
-		update_wireless_status(this, card);
 		check_access(this, card);
 		if (card->accessible && !card->wireless_disconnected && !card->emitted) {
 			int res = emit_added_object_info(this, card);
@@ -1078,28 +1177,11 @@ static int start_inotify(struct impl *this)
 	return 0;
 }
 
-static struct card *find_card_by_wireless_status_path(struct impl *this, const char *syspath)
-{
-	char wireless_path[PATH_MAX];
-	int res;
-
-	res = spa_scnprintf(wireless_path, sizeof(wireless_path), "%s/wireless_status", syspath);
-	if (res < 0 || (size_t)res >= sizeof(wireless_path))
-		return NULL;
-
-	for (unsigned int i = 0; i < this->n_cards; i++) {
-		if (spa_streq(this->cards[i].wireless_status_path, wireless_path))
-			return &this->cards[i];
-	}
-	return NULL;
-}
-
 static void impl_on_fd_events(struct spa_source *source)
 {
 	struct impl *this = source->data;
 	struct udev_device *udev_device;
-	const char *action, *subsystem, *syspath;
+	const char *action;
-	struct card *card;
 
 	udev_device = udev_monitor_receive_device(this->umonitor);
 	if (udev_device == NULL)
@@ -1109,23 +1191,12 @@ static void impl_on_fd_events(struct spa_source *source)
 	if (action == NULL)
 		action = "change";
 
-	subsystem = udev_device_get_subsystem(udev_device);
-
 	start_inotify(this);
 
-	if (spa_streq(subsystem, "sound")) {
+	if (spa_streq(action, "add") || spa_streq(action, "change"))
-		if (spa_streq(action, "add") || spa_streq(action, "change"))
+		process_udev_device(this, ACTION_CHANGE, udev_device);
-			process_udev_device(this, ACTION_CHANGE, udev_device);
+	else if (spa_streq(action, "remove"))
-		else if (spa_streq(action, "remove"))
+		process_udev_device(this, ACTION_REMOVE, udev_device);
-			process_udev_device(this, ACTION_REMOVE, udev_device);
-	} else if (spa_streq(subsystem, "hid") || spa_streq(subsystem, "usb")) {
-		if (spa_streq(action, "change")) {
-			syspath = udev_device_get_syspath(udev_device);
-			card = find_card_by_wireless_status_path(this, syspath);
-			if (card)
-				process_card(this, ACTION_CHANGE, card);
-		}
-	}
 
 	udev_device_unref(udev_device);
 }
@@ -1143,10 +1214,6 @@ static int start_monitor(struct impl *this)
 
 	udev_monitor_filter_add_match_subsystem_devtype(this->umonitor,
 							"sound", NULL);
-	udev_monitor_filter_add_match_subsystem_devtype(this->umonitor,
-							"hid", NULL);
-	udev_monitor_filter_add_match_subsystem_devtype(this->umonitor,
-							"usb", NULL);
 	udev_monitor_enable_receiving(this->umonitor);
 
 	this->source.func = impl_on_fd_events;