Functional UART logging both inside and outside dev container

[nicka101]

Currently the docker image doesn't include minicom (or sudo), leaving the user to use the script outside the dev container. This PR:

• Adds minicom and sudo to the docker image
• Switches the shebang line of uart_log.sh to bash. The -n option to read requires bash, and without this fix the script works inside the container (assuming you applied Dockerfile changes), but always selects the left bud

[Ralim]

Id prefer we just use bestool to monitor then drag in more tools? I don't see why sudo is required?

[nicka101]

Id prefer we just use bestool to monitor then drag in more tools?

For device specific functionality, I'd tend to agree with you, but rolling our own serial monitor (and including it in bestool) seems somewhat unneccessary given that:

• Writing/improving a serial monitor isn't a primary goal of this project
• It's easily outsourced to an existing fully featured program like minicom which has features like scrollback, captures, xmodem (for downloading crashdumps)
• The minicom package is 280KiB
• Having the functionality in bestool doesn't provide any additional utility, its just more code to maintain

I don't see why sudo is required?

sudo isn't actually required, it was in the change for #99 as the user outside the dev container isn't likely to be root, and binding to a serial device usually requires elevated privileges. I included it in here to maintain compatibility outside the container while essentially being a noop inside, though realistically you could just create /usr/local/bin/sudo in the container with content like

#!/usr/bin/env sh
$@

or change the uart_log.sh logic to check $EUID and achieve the same thing

[Ralim]

There already is a monitor in bestool, as well as a flash-and-monitor that avoids closing the port so you don't miss any boot errors. I got about halfway through doing the modem crashdump transfer but haven't finished it. Do intend to bring in the package to finish that. Ideally want to allow it to know about the map files. (You know, when infinite time arrives).

Not against this though. Happy to merge it. I didn't know as I had never used the vendor scripts for watching 🤣 (which is also why some stuff is a mix of ACMx and USBx as depends o nwhat device your working on)

I normally recommend having a group to own serial ports rather than sudo'ing up random tools. Don't overly mind either way.

TL;DR I'm happy to approve this, but my long term goal is to not need mincom etc and to make bestool more seamless for development.

(Apologies for not quoting, on phone 😞)

[nicka101]

Up to you guys how you want to manage it tbh.

I agree that features like flash and monitor is handy, and by all means down the line when the bestool monitor is at/above feature parity I think removing minicom and its references altogether is reasonable, it's one of the reason its in its own RUN directive in the Dockerfile (so it forms its own build layer, and can easily be excluded later without requiring a full image rebuild)

I normally recommend having a group to own serial ports rather than sudo'ing up random tools. Don't overly mind either way.

Probably a good idea, I just haven't got round to it, and figured sudo was a simpler solution, and maybe quicker for new users to get started

If you want me to change the sudo package for something like the wrapper script I suggested above, let me know (or feel free to do it yourself)

I got about halfway through doing the modem crashdump transfer but haven't finished it. Do intend to bring in the package to finish that

Minicom depends on lrzsz for its modem transfer, you could always go that route and call out to an external tool

I didn't know as I had never used the vendor scripts for watching 🤣 (which is also why some stuff is a mix of ACMx and USBx as depends o nwhat device your working on)

Minicom is decent for debugging, it helped quite a lot with debugging LDAC and finding the bug in the frequency selection, maybe give it a try. On that note, my changes in #99 assumed that the charger always identifies as usb-wch.cn_USB_Dual_Serial_0123456789, which I think is a fairly safe bet. (id is determined by the iManufacturer, iProduct, and iSerial fields): Perhaps you could confirm that yours identifies the same and if so use those device paths as the defaults in bestool (and/or the other vendor scripts)?

Left: /dev/serial/by-id/usb-wch.cn_USB_Dual_Serial_0123456789-if02 Right: /dev/serial/by-id/usb-wch.cn_USB_Dual_Serial_0123456789-if00 (Please note that /dev/serial and its subfolders only exists while at least 1 serial device is connected)

[shymega]

I'm happy to merge this for now, but long-term, I want to use bestool.

[shymega]

Also, the /dev/serial paths I think will depend on the serial number of the USB device. I will check my set in a moment.

[nicka101]

I'm happy to merge this for now, but long-term, I want to use bestool.

I agree. I don't think code changes generally should be considered from the perspective of "we'll keep this forever", particularly when its a small change and it doesn't meaningfully increase the maintenance load. My focus here, as it was with #99, is just to have full functionality for debugging now to avoid this project stalling waiting for improvements to a separate tool Long term having an all-in-one helper tool that covers all the functionality is useful, but currently it does not, and to quote Ralim:

You know, when infinite time arrives

It's apparent it has no defined release date, so we should have something that does cover the missing functionality in the meantime

Also, the /dev/serial paths I think will depend on the serial number of the USB device. I will check my set in a moment.

As I mentioned above, they do depend on serial number, but given that on mine it's set to 0123456789 (clearly not a unique number), I'm assuming that to be the case on all PineBuds, but would be good to confirm

[nicka101]

If they report a different serial number, the invocation could always be changed to use shell globbing to pick the correct device i.e.: rightbud=/dev/serial/by-id/usb-wch.cn_USB_Dual_Serial_*-if00

[shymega]

Hi Nick,

On 11.05.2024 09:50, Nick Anstee wrote:

I'm happy to merge this for now, but long-term, I want to use bestool.

I agree. I don't think code changes generally should be considered from the perspective of "we'll keep this forever", particularly when its a small change and it doesn't meaningfully increase the maintenance load. My focus here, as it was with #99, is just to have full functionality for debugging now to avoid this project stalling waiting for improvements to a separate tool

Long term having an all-in-one helper tool that covers all the functionality

is useful, but currently it does not, and to quote Ralim:

You know, when infinite time arrives

It's apparent it has no defined release date, so we should have something that does cover the missing functionality in the meantime

Agreed, there's probably not much harm in merging this at the moment.

bestool is being rewritten by myself. Help is welcome, but I'm also refactoring OPB to use CMake as well. I was thinking of creating a 'call for help' to help translate the Makefiles to CMake, into the cmake branch. There's a lot of Makefiles to go through, and automated solutions are messy.

Also, the /dev/serial paths I think will depend on the serial number of the USB device. I will check my set in a moment.

As I mentioned above, they do depend on serial number, but given that on mine it's set to 0123456789 (clearly not a unique number), I'm assuming that to be the case on all PineBuds, but would be good to confirm

Yes, I've just tested my PineBuds. I get the same path. I think it'd be worth using $(uname -s) to figure out if we're running on macOS or BSD in the future though, so we don't assume we're using Linux.

I believe /dev/serial/by-id/ nodes are a Linux-specific feature, right? As we use a Docker container based on Linux, this probably doesn't matter so much.

I don't know how @Ralim feels about it, but I would be happy to merge a PR adding /dev/serial/by-id/ support to the shell scripts. We just need to test that the order of the buds inserted into the case doesn't matter.

Thank you, for your contributions. You've asked questions that provoke interesting discussions, and created PRs - I just wanted to express my gratitude.

Best wishes,

Dom Rodriguez GPG Fingerprint: EB0D 45E6 D0DC 1BA1 A2B5 FC24 72DC F123 1E54 BD43

[nicka101]

bestool is being rewritten by myself. Help is welcome, but I'm also refactoring OPB to use CMake as well. I was thinking of creating a 'call for help' to help translate the Makefiles to CMake, into the cmake branch. There's a lot of Makefiles to go through, and automated solutions are messy.

I'd be happy to help out, but its been a while since I used CMake, so my focus so far has been on relatively simple/easy wins like slightly more convenient debugging (#99 and this) and improvements over the stock firmware (#100).

I maintain quite a lot of old/legacy code for work, so I understand the desire to refactor, but that said, do you have any particular reason/goal in mind with the change of build system from GNU Make to CMake?

Yes, I've just tested my PineBuds. I get the same path

Good to know

I believe /dev/serial/by-id/ nodes are a Linux-specific feature, right?

They're a feature of Linux's udev above kernel version 2.5 (released in 2001), and configured by the default rules in /lib/udev/rules.d/60-serial.rules, more specifically this line:

ENV{.ID_PORT}=="", SYMLINK+="serial/by-id/$env{ID_BUS}-$env{ID_SERIAL}-if$env{ID_USB_INTERFACE_NUM}"

As we use a Docker container based on Linux, this probably doesn't matter so much.

Well, it's not quite that simple, a container has no kernel and no default access to devices because it isn't managing them, the host kernel is (hence why the runtime needs to be explicitly told --privileged or handed /dev directly). I'm guessing that Docker on MacOS/BSD works similarly to Docker desktop on windows (or WSL), and spawns a VM running a linux kernel to hold the containers (essentially becoming a type 2 hypervisor), in which case it should work there too, however I don't have a machine to hand with MacOS/BSD on it to check

I don't know how @Ralim feels about it, but I would be happy to merge a PR adding /dev/serial/by-id/ support to the shell scripts. We just need to test that the order of the buds inserted into the case doesn't matter.

You can see in the udev rules its based on the case's USB descriptor, so -if00 is always the right and -if02 always the left. Like the ttyACM devices (which the paths in /dev/serial symlink to), the devices show up based on the presence of the case, not dependent on whether the buds are actually inserted

Thank you, for your contributions. You've asked questions that provoke interesting discussions, and created PRs - I just wanted to express my gratitude.

No problem, just trying get the PineBuds and this firmware to the point that I can recommend them to friends as a cheap alternative to the major brands :)

[shymega]

On 11.05.2024 11:15, Nick Anstee wrote:

bestool is being rewritten by myself. Help is welcome, but I'm also refactoring OPB to use CMake as well. I was thinking of creating a 'call for help' to help translate the Makefiles to CMake, into the cmake branch. There's a lot of Makefiles to go through, and automated solutions are messy.

I'd be happy to help out, but its been a while since I used CMake, so my focus so far has been on relatively simple/easy wins like slightly more convenient debugging (#99 and this) and improvements over the stock firmware (#100).

I maintain quite a lot of old/legacy code for work, so I understand the desire to refactor, but that said, do you have any particular reason/goal in mind with the change of build system from GNU Make to CMake?

If you have a look at the Makefiles in the repo, it's pretty messy. It also makes autocomplete and LSP features with compile_commands.json unworkable without an external tool, like Bear.

CMake would be easier to maintain, and we also are considering using Rust for some aspects of OPB, and to use a different RTOS. At the moment, the Makefiles are too rigid to do this, and too convoluted.

CMake would also lower the barrier to entry, and integrates well with Rust.

Yes, I've just tested my PineBuds. I get the same path

Good to know

I believe /dev/serial/by-id/ nodes are a Linux-specific feature, right?

They're a feature of Linux's udev above kernel version 2.5 (released in 2001), and configured by the default rules in /lib/udev/rules.d/60-serial.rules, more specifically this line:

ENV{.ID_PORT}=="",
SYMLINK+="serial/by-id/$env{ID_BUS}-$env{ID_SERIAL}-if$env{ID_USB_INTERFACE_NUM}"

OK. Given that's been around since 2001 (jeez, I feel old... I started with Linux 2.6.32), we can assume it'll be available. Good stuff.

As we use a Docker container based on Linux, this probably doesn't matter so much.

Well, it's not quite that simple, a container has no kernel and no default access to devices because it isn't managing them, the host kernel is (hence why the runtime needs to be explicitly told --privileged or handed /dev directly). I'm guessing that Docker on MacOS/BSD works similarly to Docker desktop on windows (or WSL), and spawns a VM running a linux kernel to hold the containers (essentially becoming a type 2 hypervisor), in which case it should work there too, however I don't have a machine to hand with MacOS/BSD on it to check

Oh, yes, you're quite right there. Sorry. I'm a bit out of it today.

All valid points.

I don't know how @.***(https://github.com/Ralim) feels about it, but I would be happy to merge a PR adding /dev/serial/by-id/ support to the shell scripts. We just need to test that the order of the buds inserted into the case doesn't matter.

You can see in the udev rules its based on the case's USB descriptor, so -if00 is always the right and -if02 always the left. Like the ttyACM devices (which the paths in /dev/serial symlink to), the devices show up based on the presence of the case, not dependent on whether the buds are actually inserted

That is great to know. We can plan to use that in bestool too.

Thank you, for your contributions. You've asked questions that provoke interesting discussions, and created PRs - I just wanted to express my gratitude.

No problem, just trying get the PineBuds and this firmware to the point that I can recommend them to friends as a cheap alternative to the major brands :)

It'll be a while yet, and my advice to all non-devs is to use stock firmware.

Best wishes,

Dom Rodriguez GPG Fingerprint: EB0D 45E6 D0DC 1BA1 A2B5 FC24 72DC F123 1E54 BD43

[Ralim]

I'm going to merge this as is; I do agree its a win.

The ID's should be the same for all production devices (and I dont use the scripts typically so doesnt matter for me 😁 I guess its a win for the fact they dont set unique serial numbers in these uart2usb chips. If we run into issues we can just go to globbing most likely.