#2697 PR `merged`: more control over serial devices used¶

Labels: enhancement, fixed / solved / done, hacktoberfest-accepted

DEvil0000 opened issue at 2021-10-15 13:31:¶

Type: Bug Fix / Enhancement
Impact: Normal
Reference to related issue (URL): #2663 ( #2648 )
How was this pull request tested: the basic options for serial on a APU2
Brief description of the changes in this pull request:
added SERIAL_CONSOLE_DEVICES to name a list of devices instead of using all found ones
added SERIAL_CONSOLE_DEVICE_GRUB to use a specific one for grub instead of the first one
moved some potentially reusable parts in lib/serial-functions.sh
fixed a ubuntu 20.04/systemd issue if ttyS0 and tty0 was given to the kernel (tty0 was added by mistake in some cases)

Please add hacktoberfest topic to teh repo or hacktoberfest-accepted label to the PR if you are willing to merge ;)

jsmeix commented at 2021-10-18 10:06:¶

@DEvil0000
thank you for your enhancement!
I will have a look at your changes soon...

jsmeix commented at 2021-10-18 13:02:¶

@DEvil0000
I see that most of the issues I have with the code
in this pull request are inherited from our old code
and I do not like it when you need to fix our old code issues
so if you like to fix them I would much appreciate it
but I could as well just merge this pull request as is
because to me it looks as what it implements is OK
only how it is implemented looks not well to me and then
afterwards I could make the code look well to me.

Simply put:
Would you like to make the code look well
or should I merge it and then I make the code look well?

DEvil0000 commented at 2021-10-18 13:15:¶

I fixed the obvois things a typos.
The config singular this is on purpose - see comment.
The cmdline concat is something I am not sure if a missing speed return would mean the device can not get used.
The other change request would cause changes all over the code base which I consider out of scope for this PR.

Please consider adding the hacktoberfest topic to the repo or hacktoberfest-accepted label to the PR ;)

jsmeix commented at 2021-10-18 13:16:¶

@DEvil0000
just adapt your current code in this pull request as you like
and tell me when you are finished with that.
Then I will "just merge" it and do the more generic cleanup
regarding our old serial console code on my own.

I will need to sleep on it whether or not the config variables
SERIAL_CONSOLE_DEVICE_SYSLINUX and
SERIAL_CONSOLE_DEVICE_GRUB should be singular
because we cannot change config variable names with reasonable effort
so if some future GRUB or GRUB on other hardware works with
more than one console device we could not change
SERIAL_CONSOLE_DEVICE_GRUB to
SERIAL_CONSOLE_DEVICES_GRUB
(we would need additional fallback code for the old name).
So currently I think I would more prefer to have variable names like
SERIAL_CONSOLE_DEVICES_SYSLINUX and
SERIAL_CONSOLE_DEVICES_GRUB plus matching code
and have a comment in default.conf that usually only one
single serial console device is known to work in practice.

jsmeix commented at 2021-10-18 13:20:¶

@DEvil0000
I don'k now about hacktoberfest (likely I am just too busy to have noticed it)
so what do I need to do to add the hacktoberfest topic to the repo
or add a hacktoberfest-accepted label to the PR?

DEvil0000 commented at 2021-10-18 13:31:¶

The topic on the repo may 'invite' people looking/searching for open issues in hacktober to fix something and create a PR. They however expect responses in reasonable time so they get the merch/t-shirt at the end. The label at a specific PR does also make it count for hacktoberfest but does not attract random people to fix/improve things.

I don't think more then one serial device will ever work okay in any bootloader with all hardware. So I consider the PR ready to merge ;)

jsmeix commented at 2021-10-19 08:35:¶

@DEvil0000
thank you for your enhancement of ReaR!

DEvil0000 commented at 2021-10-19 08:47:¶

@jsmeix how about adding the label "hacktoberfest-accepted" so this PR will count ;)
see here https://hacktoberfest.digitalocean.com/faq

jsmeix commented at 2021-10-19 12:01:¶

Puhh - was simple as soon as one knows what one has to do.
But for beginners the documentation about hacktoberfest
is as confusing as our ReaR documentation is...

DEvil0000 commented at 2021-10-19 12:08:¶

thanks ;)
you may in addition still add the topic "hacktoberfest" to the repo so someone looking for open issues will find the repo over the search as a PR invitation basically: https://github.com/topics/hacktoberfest

jsmeix commented at 2021-10-19 12:39:¶

Phew!
Was again simple as soon as one knows what one has to do.
For now I added only hacktoberfest2021 (yes, the trailing space is needed)
Let's see what happens - hell breaks lose or hell freezes or something less scaring ;-)

[Export of Github issue for rear/rear.]

#2697 PR merged: more control over serial devices used¶