Functional UART logging both inside and outside dev container #101
Conversation
Adds minicom and sudo to the docker image Switches the shebang line to bash to fix a bug in uart_log.sh when inside the container (-n option to read requires bash)
|
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:
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 or change the uart_log.sh logic to check $EUID and achieve the same thing |
|
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 😞) |
|
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)
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)
Minicom depends on lrzsz for its modem transfer, you could always go that route and call out to an external tool
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. Left: |
|
I'm happy to merge this for now, but long-term, I want to use |
|
Also, the |
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
It's apparent it has no defined release date, so we should have something that does cover the missing functionality in the meantime
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 |
|
If they report a different serial number, the invocation could always be changed to use shell globbing to pick the correct device i.e.: |
|
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
|
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?
Good to know
They're a feature of Linux's udev above kernel version 2.5 (released in 2001), and configured by the default rules in
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
You can see in the udev rules its based on the case's USB descriptor, so
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 :) |
|
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
|
|
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. |
Currently the docker image doesn't include minicom (or sudo), leaving the user to use the script outside the dev container.
This PR:
uart_log.shto 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