Skip to content

Commit

Permalink
avoid monitoring device types without device files (#1219)
Browse files Browse the repository at this point in the history
fixes a bug in cac8479 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.
  • Loading branch information
ngwese authored Oct 29, 2020
1 parent 90ec454 commit fe416d1
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 16 deletions.
8 changes: 6 additions & 2 deletions matron/src/device/device_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
30 changes: 16 additions & 14 deletions matron/src/device/device_monitor.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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,
Expand Down Expand Up @@ -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);
}
}
Expand All @@ -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;

Expand Down Expand Up @@ -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()");
Expand All @@ -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) {
Expand All @@ -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));
}
}
Expand All @@ -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) {
Expand All @@ -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;
}
Expand Down

0 comments on commit fe416d1

Please sign in to comment.