From fe416d13704a7da632bbb57c1fae30bf9276ebf8 Mon Sep 17 00:00:00 2001 From: Greg Wuller Date: Wed, 28 Oct 2020 19:39:21 -0700 Subject: [PATCH] avoid monitoring device types without device files (#1219) fixes a bug in cac8479fcc where crow devices were mapped to the virtual midi device type (which has no corresponding device file to watch for) because the device pattern watch list did not match the order of the `device_t` enum values. --- matron/src/device/device_common.h | 8 ++++++-- matron/src/device/device_monitor.c | 30 ++++++++++++++++-------------- 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/matron/src/device/device_common.h b/matron/src/device/device_common.h index 190779473..477313a77 100644 --- a/matron/src/device/device_common.h +++ b/matron/src/device/device_common.h @@ -9,14 +9,18 @@ typedef enum { DEV_TYPE_HID = 1, // raw midi devices DEV_TYPE_MIDI = 2, - DEV_TYPE_MIDI_VIRTUAL = 3, // usbmodem (crow) - DEV_TYPE_CROW = 4, + DEV_TYPE_CROW = 3, + // place all virtual devices (devices without device files to monitor) below + DEV_TYPE_MIDI_VIRTUAL = 4, // counter - unused, don't remove DEV_TYPE_COUNT, DEV_TYPE_INVALID } device_t; +// maximum device_t value for devices which have corresponding device files +#define DEV_TYPE_COUNT_PHYSICAL 4 + struct dev_common { // device type device_t type; diff --git a/matron/src/device/device_monitor.c b/matron/src/device/device_monitor.c index ca565e7f8..de50a937d 100644 --- a/matron/src/device/device_monitor.c +++ b/matron/src/device/device_monitor.c @@ -41,13 +41,14 @@ struct watch { // watchers // FIXME: these names / paths are really arbitrary. -static struct watch w[DEV_TYPE_COUNT] = {{.sub_name = "tty", .node_pattern = "/dev/ttyUSB*"}, - {.sub_name = "input", .node_pattern = "/dev/input/event*"}, - {.sub_name = "sound", .node_pattern = "/dev/snd/midiC*D*"}, - {.sub_name = "crow", .node_pattern = "/dev/ttyACM*"}}; +static struct watch w[DEV_TYPE_COUNT_PHYSICAL] = { + {.sub_name = "tty", .node_pattern = "/dev/ttyUSB*"}, + {.sub_name = "input", .node_pattern = "/dev/input/event*"}, + {.sub_name = "sound", .node_pattern = "/dev/snd/midiC*D*"}, + {.sub_name = "crow", .node_pattern = "/dev/ttyACM*"}}; // file descriptors to watch/poll -struct pollfd pfds[DEV_TYPE_COUNT]; +struct pollfd pfds[DEV_TYPE_COUNT_PHYSICAL]; // thread for polling all the watched file descriptors pthread_t watch_tid; @@ -70,7 +71,7 @@ void dev_monitor_init(void) { udev = udev_new(); assert(udev); - for (int i = 0; i < DEV_TYPE_COUNT; i++) { + for (int i = 0; i < DEV_TYPE_COUNT_PHYSICAL; i++) { w[i].mon = udev_monitor_new_from_netlink(udev, "udev"); if (w[i].mon == NULL) { fprintf(stderr, "failed to start udev_monitor for subsystem %s, pattern %s\n", w[i].sub_name, @@ -105,7 +106,7 @@ void dev_monitor_init(void) { void dev_monitor_deinit(void) { pthread_cancel(watch_tid); - for (int i = 0; i < DEV_TYPE_COUNT; i++) { + for (int i = 0; i < DEV_TYPE_COUNT_PHYSICAL; i++) { free(w[i].mon); } } @@ -120,7 +121,7 @@ int dev_monitor_scan(void) { return 1; } - for (int i = 0; i < DEV_TYPE_COUNT; i++) { + for (int i = 0; i < DEV_TYPE_COUNT_PHYSICAL; i++) { struct udev_enumerate *ue; struct udev_list_entry *devices, *dev_list_entry; @@ -157,7 +158,7 @@ void *watch_loop(void *p) { struct udev_device *dev; while (1) { - if (poll(pfds, DEV_TYPE_COUNT, WATCH_TIMEOUT_MS) < 0) { + if (poll(pfds, DEV_TYPE_COUNT_PHYSICAL, WATCH_TIMEOUT_MS) < 0) { switch (errno) { case EINVAL: perror("error in poll()"); @@ -169,7 +170,7 @@ void *watch_loop(void *p) { } // see which monitor has data - for (int i = 0; i < DEV_TYPE_COUNT; i++) { + for (int i = 0; i < DEV_TYPE_COUNT_PHYSICAL; i++) { if (pfds[i].revents & POLLIN) { dev = udev_monitor_receive_device(w[i].mon); if (dev) { @@ -193,7 +194,7 @@ void handle_device(struct udev_device *dev) { if (node != NULL) { device_t t = check_dev_type(dev); - if (t >= 0 && t < DEV_TYPE_COUNT) { + if (t >= 0 && t < DEV_TYPE_COUNT_PHYSICAL) { dev_list_add(t, node, get_device_name(dev)); } } @@ -216,7 +217,7 @@ void handle_device(struct udev_device *dev) { } else { device_t t = check_dev_type(dev); - if (t >= 0 && t < DEV_TYPE_COUNT) { + if (t >= 0 && t < DEV_TYPE_COUNT_PHYSICAL) { if (strcmp(action, "add") == 0) { dev_list_add(t, node, get_device_name(dev)); } else if (strcmp(action, "remove") == 0) { @@ -235,8 +236,9 @@ device_t check_dev_type(struct udev_device *dev) { // for now, just get USB devices. // eventually we might want to use this same system for GPIO, &c... if (udev_device_get_parent_with_subsystem_devtype(dev, "usb", NULL)) { - for (int i = 0; i < DEV_TYPE_COUNT; i++) { - if (fnmatch(w[i].node_pattern, node, 0) == 0) { + for (int i = 0; i < DEV_TYPE_COUNT_PHYSICAL; i++) { + const char *node_pattern = w[i].node_pattern; + if (node_pattern[0] && fnmatch(node_pattern, node, 0) == 0) { t = i; break; }