Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mpremote: do not list bogus devices #14374

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bulletmark
Copy link
Contributor

On Linux, since kernel 6.7 (and now 6.8) we get the following:

$ mpremote connect list
/dev/ttyS0 None 0000:0000 None None
/dev/ttyS1 None 0000:0000 None None
/dev/ttyS2 None 0000:0000 None None
/dev/ttyS3 None 0000:0000 None None
/dev/ttyS4 None 0000:0000 None None
/dev/ttyS5 None 0000:0000 None None
/dev/ttyS6 None 0000:0000 None None
/dev/ttyS7 None 0000:0000 None None
/dev/ttyS8 None 0000:0000 None None
/dev/ttyS9 None 0000:0000 None None
/dev/ttyS10 None 0000:0000 None None
/dev/ttyS11 None 0000:0000 None None
/dev/ttyS12 None 0000:0000 None None
/dev/ttyS13 None 0000:0000 None None
/dev/ttyS14 None 0000:0000 None None
/dev/ttyS15 None 0000:0000 None None
/dev/ttyS16 None 0000:0000 None None
/dev/ttyS17 None 0000:0000 None None
/dev/ttyS18 None 0000:0000 None None
/dev/ttyS19 None 0000:0000 None None
/dev/ttyS20 None 0000:0000 None None
/dev/ttyS21 None 0000:0000 None None
/dev/ttyS22 None 0000:0000 None None
/dev/ttyS23 None 0000:0000 None None
/dev/ttyS24 None 0000:0000 None None
/dev/ttyS25 None 0000:0000 None None
/dev/ttyS26 None 0000:0000 None None
/dev/ttyS27 None 0000:0000 None None
/dev/ttyS28 None 0000:0000 None None
/dev/ttyS29 None 0000:0000 None None
/dev/ttyS30 None 0000:0000 None None
/dev/ttyS31 None 0000:0000 None None
/dev/ttyUSB0 02249F9A 10c4:ea60 Silicon Labs CP2104 USB to UART Bridge Controller

With this PR applied:

$ mpremote connect list
/dev/ttyUSB0 02249F9A 10c4:ea60 Silicon Labs CP2104 USB to UART Bridge Controller

@bulletmark
Copy link
Contributor Author

I should add that this problem only started from kernel 6.7+. Works fine (i.e. bogus devices are not listed) on Linux kernel 6.6 and earlier.

Copy link

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:    +0 +0.000% PYBV10
     mimxrt:    +0 +0.000% TEENSY40
        rp2:    +0 +0.000% RPI_PICO
       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS

Copy link

codecov bot commented Apr 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.39%. Comparing base (30a9ccf) to head (4536d8d).

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #14374   +/-   ##
=======================================
  Coverage   98.39%   98.39%           
=======================================
  Files         161      161           
  Lines       21204    21204           
=======================================
  Hits        20864    20864           
  Misses        340      340           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Gadgetoid
Copy link
Contributor

Strange, running Kernel 6.8 on a Pi 5 here and do not see this problem. I also don't have a dozen /dev/ttyS* devices listed in my system which strikes me as... odd... what platform are you running on? On Kernel <= 6.6 what do you see when you ls /dev/ttyS* vs > 6.6?

Devices prefixed ttyS* should be valid serial ports on hardware with GPIO-based ports. I don't think it's valid to exclude them here- since it's entirely possible (and more so plausible with the behaviour of the new USB functionality) to be connecting to a MicroPython device via an onboard hardware serial port.

Ironically /dev/ttyS* is allegedly filtered on the Pi (leading to this bug: pyserial/pyserial#489), though I have /dev/ttyS2 on a Rock 5B which is a valid hardware port that would be erroneously hidden by this change.

I think your issue is some bug in your device tree config that's enabling 32, non-existent hardware serial ports...

@bulletmark
Copy link
Contributor Author

I'm on a stock Arch x86_64 system with stock kernel etc. I have used mpremote forever and have never seen this problem until recently so I went looking today and noticed that if I switched to Arch LTS kernel (currently 6.6.28-1) then the problem goes away. Current stock kernel on Arch is 6.8.7.arch1-1 where the problem is seen but I also tried the previous stock 6.7.9.arch1-1 and the problem is seen there also. I already checked:

$ ls -l /dev/ttyS*
crw-rw---- 1 root uucp 4, 64 Apr 25 14:28 /dev/ttyS0
crw-rw---- 1 root uucp 4, 65 Apr 25 14:28 /dev/ttyS1
crw-rw---- 1 root uucp 4, 74 Apr 25 14:28 /dev/ttyS10
crw-rw---- 1 root uucp 4, 75 Apr 25 14:28 /dev/ttyS11
crw-rw---- 1 root uucp 4, 76 Apr 25 14:28 /dev/ttyS12
crw-rw---- 1 root uucp 4, 77 Apr 25 14:28 /dev/ttyS13
crw-rw---- 1 root uucp 4, 78 Apr 25 14:28 /dev/ttyS14
crw-rw---- 1 root uucp 4, 79 Apr 25 14:28 /dev/ttyS15
crw-rw---- 1 root uucp 4, 80 Apr 25 14:28 /dev/ttyS16
crw-rw---- 1 root uucp 4, 81 Apr 25 14:28 /dev/ttyS17
crw-rw---- 1 root uucp 4, 82 Apr 25 14:28 /dev/ttyS18
crw-rw---- 1 root uucp 4, 83 Apr 25 14:28 /dev/ttyS19
crw-rw---- 1 root uucp 4, 66 Apr 25 14:28 /dev/ttyS2
crw-rw---- 1 root uucp 4, 84 Apr 25 14:28 /dev/ttyS20
crw-rw---- 1 root uucp 4, 85 Apr 25 14:28 /dev/ttyS21
crw-rw---- 1 root uucp 4, 86 Apr 25 14:28 /dev/ttyS22
crw-rw---- 1 root uucp 4, 87 Apr 25 14:28 /dev/ttyS23
crw-rw---- 1 root uucp 4, 88 Apr 25 14:28 /dev/ttyS24
crw-rw---- 1 root uucp 4, 89 Apr 25 14:28 /dev/ttyS25
crw-rw---- 1 root uucp 4, 90 Apr 25 14:28 /dev/ttyS26
crw-rw---- 1 root uucp 4, 91 Apr 25 14:28 /dev/ttyS27
crw-rw---- 1 root uucp 4, 92 Apr 25 14:28 /dev/ttyS28
crw-rw---- 1 root uucp 4, 93 Apr 25 14:28 /dev/ttyS29
crw-rw---- 1 root uucp 4, 67 Apr 25 14:28 /dev/ttyS3
crw-rw---- 1 root uucp 4, 94 Apr 25 14:28 /dev/ttyS30
crw-rw---- 1 root uucp 4, 95 Apr 25 14:28 /dev/ttyS31
crw-rw---- 1 root uucp 4, 68 Apr 25 14:28 /dev/ttyS4
crw-rw---- 1 root uucp 4, 69 Apr 25 14:28 /dev/ttyS5
crw-rw---- 1 root uucp 4, 70 Apr 25 14:28 /dev/ttyS6
crw-rw---- 1 root uucp 4, 71 Apr 25 14:28 /dev/ttyS7
crw-rw---- 1 root uucp 4, 72 Apr 25 14:28 /dev/ttyS8
crw-rw---- 1 root uucp 4, 73 Apr 25 14:28 /dev/ttyS9

and these look exactly the same regardless of which kernel I run. I run a few other Arch 64 servers, e.g. on a N100 SFF box, and on a DigitalOcean VM, and when I check them I see they also show 32 ports same as above.

Regarding your comment I don't think it's valid to exclude them here- since it's entirely possible (and more so plausible with the behaviour of the new USB functionality) to be connecting to a MicroPython device via an onboard hardware serial port. Of course I am certainly not suggesting to exclude any potentially valid serial ports. The PR only filters out those which are clearly bogus (i.e. reported above as None 0000:0000 None None).

@Gadgetoid
Copy link
Contributor

The PR only filters out those which are clearly bogus (i.e. reported above as None 0000:0000 None None).

No it doesn't. It filters out any with a serial_number of None, which includes (afaik) all hardware UARTs, since those are not USB devices.

Here's what a perfectly valid hardware UART looks like on an embedded system:

/dev/ttyAMA10 None 0000:0000 None None

@bulletmark
Copy link
Contributor Author

So when a device is connected to that port how is it reported then?

@Gadgetoid
Copy link
Contributor

So when a device is connected to that port how is it reported then?

The same. UART does nothing until you open and use it. It's just a couple of pins you can choose (or not choose) to wiggle. There's no VID/PID/serial or other identifier, since those all come from the USB->UART bridge (be it hardware or software) that's typically used in USB CDC configurations.

Try running the following on your system. This reports the subsystem, which might be a more effective way to filter:

import serial.tools.list_ports

for p in serial.tools.list_ports.comports():
    print("{} {} {} {:04x}:{:04x} {} {}".format(
                p.device,
                p.subsystem,
                p.serial_number,
                p.vid if isinstance(p.vid, int) else 0,
                p.pid if isinstance(p.pid, int) else 0,
                p.manufacturer,
                p.product,
                )
            )

I wonder if the subsystem has somehow changed on your system from "platform" to "serial-base", in my case I see:

/dev/ttyAMA10 serial-base n/a None 0000:0000 None None

@bulletmark
Copy link
Contributor Author

On kernel 6.6 that prints out nothing. On kernel 6.8:

$ python tmp.py 
/dev/ttyS0 serial-base None 0000:0000 None None
/dev/ttyS1 serial-base None 0000:0000 None None
/dev/ttyS2 serial-base None 0000:0000 None None
/dev/ttyS3 serial-base None 0000:0000 None None
/dev/ttyS5 serial-base None 0000:0000 None None
/dev/ttyS6 serial-base None 0000:0000 None None
/dev/ttyS7 serial-base None 0000:0000 None None
/dev/ttyS8 serial-base None 0000:0000 None None
/dev/ttyS9 serial-base None 0000:0000 None None
/dev/ttyS10 serial-base None 0000:0000 None None
/dev/ttyS11 serial-base None 0000:0000 None None
/dev/ttyS12 serial-base None 0000:0000 None None
/dev/ttyS13 serial-base None 0000:0000 None None
/dev/ttyS14 serial-base None 0000:0000 None None
/dev/ttyS15 serial-base None 0000:0000 None None
/dev/ttyS16 serial-base None 0000:0000 None None
/dev/ttyS17 serial-base None 0000:0000 None None
/dev/ttyS18 serial-base None 0000:0000 None None
/dev/ttyS19 serial-base None 0000:0000 None None
/dev/ttyS20 serial-base None 0000:0000 None None
/dev/ttyS21 serial-base None 0000:0000 None None
/dev/ttyS22 serial-base None 0000:0000 None None
/dev/ttyS23 serial-base None 0000:0000 None None
/dev/ttyS24 serial-base None 0000:0000 None None
/dev/ttyS25 serial-base None 0000:0000 None None
/dev/ttyS26 serial-base None 0000:0000 None None
/dev/ttyS27 serial-base None 0000:0000 None None
/dev/ttyS28 serial-base None 0000:0000 None None
/dev/ttyS29 serial-base None 0000:0000 None None
/dev/ttyS30 serial-base None 0000:0000 None None
/dev/ttyS31 serial-base None 0000:0000 None None
/dev/ttyS4 serial-base None 0000:0000 None None

@Gadgetoid
Copy link
Contributor

Gadgetoid commented Apr 25, 2024

~Your ports didn't get renamed across Kernels from /dev/ttyXX to /dev/ttySXX did they? On Pi (Debian based) they are just /dev/ttyXX.

list_ports.comports() explicitly globs /dev/ttyS*, calling them "built in serial ports." See: https://github.com/pyserial/pyserial/blob/7aeea35429d15f3eefed10bbb659674638903e3a/serial/tools/list_ports_linux.py#L91-L100

I've no idea whether this is actually correct since their only source is 23 years out of date and I wont pretend to know the idiosyncrasies of UNIX, Linux and POSIX- https://web.archive.org/web/20010205041900/http://www.easysw.com/~mike/serial/serial.html~

Edit: Sorry I re-read your reply above, tty* vs ttyS* is me barking up the wrong tree. I've confirmed with some other Arch users and slightly updated the code below to reflect this-

I've plucked out the code for comports() and removed the filter on "platform" devices so we can get a better idea of what's actually changed, on both kernels try:

import glob
import serial.tools.list_ports_common
from serial.tools.list_ports_linux import SysFS


def comports(include_links=False):
    devices = set()
    devices.update(glob.glob('/dev/ttyS*'))     # built-in serial ports
    devices.update(glob.glob('/dev/ttyUSB*'))   # usb-serial with own driver
    devices.update(glob.glob('/dev/ttyXRUSB*')) # xr-usb-serial port exar (DELL Edge 3001)
    devices.update(glob.glob('/dev/ttyACM*'))   # usb-serial with CDC-ACM profile
    devices.update(glob.glob('/dev/ttyAMA*'))   # ARM internal port (raspi)
    devices.update(glob.glob('/dev/rfcomm*'))   # BT serial devices
    devices.update(glob.glob('/dev/ttyAP*'))    # Advantech multi-port serial controllers
    devices.update(glob.glob('/dev/ttyGS*'))    # https://www.kernel.org/doc/Documentation/usb/gadget_serial.txt

    if include_links:
        devices.update(list_ports_common.list_links(devices))
    return [SysFS(d) for d in devices]


for p in comports():
    print("{} {} {} {:04x}:{:04x} {} {}".format(
                p.device,
                p.subsystem,
                p.serial_number,
                p.vid if isinstance(p.vid, int) else 0,
                p.pid if isinstance(p.pid, int) else 0,
                p.manufacturer,
                p.product,
                )
            )

@Gadgetoid
Copy link
Contributor

Note: I've got independent confirmation that the 6.6 -> 6.8 Kernel transition does indeed change the subsystem of these nodes from "platform" to "serial-base" which, afaik, is a bug in the Arch kernel/system and not this tool. I haven't got the first clue where this change and how to report this as a bug, though...

@bulletmark
Copy link
Contributor Author

The output from that second script on 6.8 is exactly the same as the previous output I pasted here. On 6.6, it is also the same except platform is shown instead of serial-base for every device.

Note Arch is a distro and merely packages up the stock Linux kernel so this is not a bug in Arch. Report it to Linus :).

This is clearly a bug somewhere (possibly pyserial needs an update for kernels >= 6.7?) but regardless I don't see why mpremote would ever want to list 32 ports with no device information (even if there is actually a device connected) so this PR may still be considered valid by the mpremote developers.

bulletmark added a commit to bulletmark/mpr that referenced this pull request Apr 25, 2024
This change needed on systems with Linux kernel >= 6.7.

This is likely a temporary change until the issue discussed in
micropython/micropython#14374 is addressed.
@Gadgetoid
Copy link
Contributor

The output from that second script on 6.8 is exactly the same as the previous output I pasted here. On 6.6, it is also the same except platform is shown instead of serial-base for every device.

So Arch has made a change to cause this. Sounds an awful lot like an Arch problem and not a the-way-serial-ports-have-been-enumerated-for-25-years-needs-to-change problem... 😆

Note Arch is a distro and merely packages up the stock Linux kernel so this is not a bug in Arch

Then why does my 6.8 Raspberry Pi kernel not have the same issue? I guess the short answer to this is that Debian uses /dev/tty* rather than /dev/ttyS* so the ttys don't leak into UART enumeration at all... but it's notable that they also a platform of None to further disambiguate them.

In Arch, it could easily be a configuration issue that's surfaced by a Kernel change- but again, I've no earthly clue where to locate what actually changed and why.

Since "platform" vs "serial-base" is, afaict, the canonical way to tell /dev/ttyS* UARTs from ttys I don't have any grand ideas how to fix your problem without breaking serial port enumeration for embedded users...

(I realise my tone probably comes across as "some guy being awkward for the sake of awkward" here, and maybe in some way I am- but I promise I'm just trying to get to the actual root issue because this change might affect me, or our production product provisioning process. P.S. I don't have anything against Arch)

Anyway I've stated my case, we've got to the root cause, and - indeed - we probably need some guidance from actual maintainers on this 😆

@bulletmark
Copy link
Contributor Author

@Gadgetoid what kernel version are you running on your RPi? And what output do you get from mpremote devs with a MP device connected on a UART port, and without any MP devices connected?

@projectgus
Copy link
Contributor

projectgus commented Apr 29, 2024

I see the same behaviour on kernel 6.8.7-arch1-1, but agree this unfortunately isn't something we can fix in mpremote. If the operating system reports 32 bogus hardware serial ports then this is a problem of the operating system, not mpremote. There are definitely legitimate reasons for using mpremote with hardware serial ports.

@bulletmark
Copy link
Contributor Author

@projectgus I think you and Gadgetoid are misunderstanding this PR. It does not prohibit mpremote from connecting to UART ports. You can specify any port you like and mpremote will use it. All this PR does is stop those ports from being listed in the output of mpremote devs. Users run that command when they are trying to see what and where they have devices connected. What is the point of listing all UART serial ports on your system every time (even if one of them actually does has a MP device connected)? E.g, without this PR:
`
$ mpremote devs
/dev/ttyS0 None 0000:0000 None None
/dev/ttyS1 None 0000:0000 None None
/dev/ttyS2 None 0000:0000 None None
/dev/ttyS3 None 0000:0000 None None
/dev/ttyS4 None 0000:0000 None None
/dev/ttyS5 None 0000:0000 None None
/dev/ttyS6 None 0000:0000 None None
/dev/ttyS7 None 0000:0000 None None
/dev/ttyS8 None 0000:0000 None None
/dev/ttyS9 None 0000:0000 None None
/dev/ttyS10 None 0000:0000 None None
/dev/ttyS11 None 0000:0000 None None
/dev/ttyS12 None 0000:0000 None None
/dev/ttyS13 None 0000:0000 None None
/dev/ttyS14 None 0000:0000 None None
/dev/ttyS15 None 0000:0000 None None
/dev/ttyS16 None 0000:0000 None None
/dev/ttyS17 None 0000:0000 None None
/dev/ttyS18 None 0000:0000 None None
/dev/ttyS19 None 0000:0000 None None
/dev/ttyS20 None 0000:0000 None None
/dev/ttyS21 None 0000:0000 None None
/dev/ttyS22 None 0000:0000 None None
/dev/ttyS23 None 0000:0000 None None
/dev/ttyS24 None 0000:0000 None None
/dev/ttyS25 None 0000:0000 None None
/dev/ttyS26 None 0000:0000 None None
/dev/ttyS27 None 0000:0000 None None
/dev/ttyS28 None 0000:0000 None None
/dev/ttyS29 None 0000:0000 None None
/dev/ttyS30 None 0000:0000 None None
/dev/ttyS31 None 0000:0000 None None

Yep, I can see that I have 32 serial ports. Possibly there is an MP device connected to one of them but the above output is pointless.

@projectgus
Copy link
Contributor

It looks like Arch changed from 4 runtime UARTs to 32 a while back in their kernel configuration: https://bbs.archlinux.org/viewtopic.php?id=271561

As noted in that thread, there's a kernel command line parameter that lets you turn this value down.

So I think this is both an x86_64 thing, and probably an Arch-specific thing.

I can't explain why the 32 UARTs apparently didn't show on the arch-lts kernel, as the config suggests it's the same there too:
https://gitlab.archlinux.org/archlinux/packaging/packages/linux-lts/-/blob/main/config?ref_type=heads&blame=1#L4564

Maybe on some generic x86 systems there wasn't an 8250 driver being loaded until recently, that might be the root cause there.

@projectgus
Copy link
Contributor

projectgus commented Apr 29, 2024

All this PR does is stop those ports from being listed in the output of mpremote devs. Users run that command when they are trying to see what and where they have devices connected.

That is a worthwhile distinction, thanks @bulletmark . Perhaps a more specific description of what this PR does is:

  • Change mpremote connect list to only list USB serial devices.

The additional metadata is all USB related, so that's effectively what this change does.

There is still a downside from this, for example if you're on a system where you don't remember what the hardware serial ports are named (i.e. /dev/ttyAMAx on ARM) then you might reasonably expect mpremote connect list to show this information.

More generally, I'd suggest the principle of least surprise here is that "mpremote connect list" lists the allowed values that can be supplied to "mpremote connect". That's symmetrical, at least. So it should include any hardware ports that the operating system says are there. It's unfortunate and kind of weird that there doesn't seem to be a way to tell if a hardware serial port is "bogus" without opening it (which immediately fails, at least for all the ttyS devices on my serial-port-free laptop!)

@dpgeorge
Copy link
Member

I'm also seeing this issue on Arch Linux. It's annoying, but...

I also see the same problem now with Chromium's available list of serial devices when using WebSerial (on Arch Linux, with Chromium). When you attempt to open a WebSerial connection, the Chromium pop-up dialog presents you with 32 "bogus" tty devices.

So that makes me think it's really an Arch Linux "bug".

@bulletmark
Copy link
Contributor Author

@dpgeorge it is only a problem on Arch because Arch uses bleeding edge Linux kernel versions and it only happened on the update of the kernel to 6.7 (6.6 and earlier are fine). Thus it will likely be a problem of all other distros when they catch up. At the end of the day this PR is a trivial one-line if test and just skips clearly bogus devices from being output in the mpremote devs list which, regardless of anything else, surely seems a sensible change?

Signed-off-by: Mark Blakeney <mark.blakeney@bullet-systems.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants