diff --git a/README.md b/README.md index 8d7281d..e18132e 100644 --- a/README.md +++ b/README.md @@ -155,7 +155,7 @@ telling ModemManager to leave the port alone is cleaner — `./install.sh --udev or by hand: ```bash -sudo cp packaging/99-skydimo.rules /etc/udev/rules.d/ +sudo cp packaging/70-skydimo.rules /etc/udev/rules.d/ sudo udevadm control --reload-rules && sudo udevadm trigger ``` diff --git a/install.sh b/install.sh index 729a2df..f37cd2d 100755 --- a/install.sh +++ b/install.sh @@ -88,7 +88,7 @@ do_uninstall() { ok "removed program files, service and launcher" say "" say "Your settings were left at $CONFDIR/config.json" - say "The udev rule, if you installed it, is at /etc/udev/rules.d/99-skydimo.rules" + say "The udev rule, if you installed it, is at /etc/udev/rules.d/70-skydimo.rules" exit 0 } @@ -334,13 +334,17 @@ command -v update-desktop-database >/dev/null 2>&1 && \ update-desktop-database "$DESKTOPDIR" 2>/dev/null || true ok "desktop entry: $DESKTOPDIR/$APP_NAME.desktop" -cp "$SRC_DIR/packaging/99-skydimo.rules" "$PREFIX/" +cp "$SRC_DIR/packaging/70-skydimo.rules" "$PREFIX/" +rm -f "$PREFIX/99-skydimo.rules" cp "$SRC_DIR/README.md" "$PREFIX/" 2>/dev/null || true if [ "$DO_UDEV" = 1 ]; then step "Installing the udev rule" - if sudo install -m 0644 "$SRC_DIR/packaging/99-skydimo.rules" \ - /etc/udev/rules.d/99-skydimo.rules; then + # Older versions installed it as 99-, which sorts after 73-seat-late.rules + # and so never got the uaccess ACL. + if sudo install -m 0644 "$SRC_DIR/packaging/70-skydimo.rules" \ + /etc/udev/rules.d/70-skydimo.rules; then + sudo rm -f /etc/udev/rules.d/99-skydimo.rules sudo udevadm control --reload-rules && sudo udevadm trigger || true ok "ModemManager will leave the adapter alone" else diff --git a/packaging/70-skydimo.rules b/packaging/70-skydimo.rules new file mode 100644 index 0000000..573b853 --- /dev/null +++ b/packaging/70-skydimo.rules @@ -0,0 +1,14 @@ +# Keep ModemManager away from the Skydimo adapter's CH340 bridge, hand it to the +# logged-in user, and name it /dev/skydimo so the driver never has to guess +# which serial port is the strip. Without the ModemManager part, it probes the +# port with AT commands for a few seconds after plug-in, which can collide with +# the driver's handshake. +# +# The 70- prefix matters: TAG+="uaccess" is acted on by 73-seat-late.rules, so a +# rule sorted after it (99-...) sets the tag too late and the user gets no ACL. +# Install with: +# sudo cp ~/.local/share/skydimo/70-skydimo.rules /etc/udev/rules.d/ +# sudo udevadm control --reload-rules && sudo udevadm trigger +SUBSYSTEM=="tty", ATTRS{idVendor}=="1a86", ATTRS{idProduct}=="7523", \ + ENV{ID_MM_DEVICE_IGNORE}="1", ENV{ID_MM_PORT_IGNORE}="1", TAG+="uaccess", \ + SYMLINK+="skydimo" diff --git a/packaging/99-skydimo.rules b/packaging/99-skydimo.rules deleted file mode 100644 index 0eccbe1..0000000 --- a/packaging/99-skydimo.rules +++ /dev/null @@ -1,8 +0,0 @@ -# Keep ModemManager away from the Skydimo adapter's CH340 bridge. Without this, -# ModemManager probes the port with AT commands for a few seconds after plug-in, -# which can collide with the driver's handshake. -# Install with: -# sudo cp ~/.local/share/skydimo/99-skydimo.rules /etc/udev/rules.d/ -# sudo udevadm control --reload-rules && sudo udevadm trigger -SUBSYSTEM=="tty", ATTRS{idVendor}=="1a86", ATTRS{idProduct}=="7523", \ - ENV{ID_MM_DEVICE_IGNORE}="1", ENV{ID_MM_PORT_IGNORE}="1", TAG+="uaccess" diff --git a/src/skydimo/device.py b/src/skydimo/device.py index d7ad266..a39bb23 100644 --- a/src/skydimo/device.py +++ b/src/skydimo/device.py @@ -37,6 +37,7 @@ from __future__ import annotations import glob +import os import time import serial @@ -45,6 +46,7 @@ BAUD = 1_000_000 VID_CH340 = 0x1A86 PID_CH340 = 0x7523 +UDEV_SYMLINK = "/dev/skydimo" # created by packaging/70-skydimo.rules HDR_MAGIC0 = 0x54 # 'T' HDR_MAGIC1 = 0x6E # 'n' @@ -68,14 +70,18 @@ class HandshakeError(RuntimeError): def find_port() -> str: """Locate the adapter's serial port (CH340 bridge).""" + # The udev rule names the adapter, so this never has to guess. + if os.path.exists(UDEV_SYMLINK): + return UDEV_SYMLINK for p in list_ports.comports(): if p.vid == VID_CH340 and p.pid == PID_CH340: return p.device - # Fall back to any USB serial node; the handshake will confirm identity. - for pattern in ("/dev/ttyUSB*", "/dev/ttyACM*"): - found = sorted(glob.glob(pattern)) - if found: - return found[0] + # Fall back to USB serial bridges only. The CH340 never enumerates as + # ttyACM, and ttyACM nodes are other people's CDC devices (LCD panels, + # microcontrollers): the DTR reset and 1 Mbaud handshake would disrupt them. + found = sorted(glob.glob("/dev/ttyUSB*")) + if found: + return found[0] raise FileNotFoundError( "No USB serial adapter found. Is the Skydimo adapter plugged in?" ) diff --git a/tests/test_core.py b/tests/test_core.py index 12ebb88..a950062 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -314,5 +314,27 @@ def test_sampling_picks_up_edge_position(self): self.assertGreater(top.mean(), bottom.mean() + 10) +class TestFindPort(unittest.TestCase): + def _find(self, exists=False, comports=(), globbed=()): + from unittest import mock + from skydimo import device + with mock.patch.object(device.os.path, "exists", return_value=exists), \ + mock.patch.object(device.list_ports, "comports", return_value=list(comports)), \ + mock.patch.object(device.glob, "glob", + side_effect=lambda pat: [g for g in globbed if g.startswith(pat[:-1])]): + return device.find_port() + + def test_prefers_udev_symlink(self): + self.assertEqual(self._find(exists=True, globbed=["/dev/ttyUSB0"]), "/dev/skydimo") + + def test_never_falls_back_to_ttyacm(self): + # e.g. an LCD panel on ttyACM0 with no Skydimo adapter plugged in + with self.assertRaises(FileNotFoundError): + self._find(globbed=["/dev/ttyACM0"]) + + def test_falls_back_to_ttyusb(self): + self.assertEqual(self._find(globbed=["/dev/ttyACM0", "/dev/ttyUSB1"]), "/dev/ttyUSB1") + + if __name__ == "__main__": unittest.main()