Convert shell scripts to not be Bash-specific about 8821cu-20210916

[alkisg]

@morrownr:

There has got to be a sh-compatible way to do the editor stuff but it escapes me what it is right now.

Initial code:

DEFAULT_EDITOR="$(<default-editor.txt)"
EDITORS_SEARCH=("${VISUAL}" "${EDITOR}" "${DEFAULT_EDITOR}" "vi")
# Try to find the user's default text editor through the EDITORS_SEARCH array
for editor in ${EDITORS_SEARCH[@]}
do
	if command -v "${editor}" >/dev/null 2>&1
	then
		TEXT_EDITOR="${editor}"
		break
	fi
done

A proposal that aims for minimal changes:

DEFAULT_EDITOR="$(cat default-editor.txt)"
EDITORS_SEARCH="${VISUAL} ${EDITOR} ${DEFAULT_EDITOR} vi"
# Try to find the user's default text editor through the EDITORS_SEARCH array
for editor in ${EDITORS_SEARCH}
do
	if command -v "${editor}" >/dev/null 2>&1
	then
		TEXT_EDITOR="${editor}"
		break
	fi
done

An alternative proposal that tranforms that into an edit file function:

# Edit a file with the user's selected editor
edit() {
	# Cache the result for speed
	if [ -z "$editor" ]; then
		for editor in $(cat default-editor.txt 2>/dev/null) $VISUAL $EDITOR vi; do
			command -v "$editor" >/dev/null 2>&1 && break
		done
	fi
	"$editor" "$@"
}

Btw, not sure where "default-editor.txt" is supposed to live, in the current directory? Even when using dkms?

[M0les]

I think a mishmash of the two solutions from @alkisg might do.

Basically arrays are Bash specific, but you can iterate over a set of space-separated strings. While you can put those into a variable, there's no way to escape embedded spaces or other fun stuff. So maybe just ditch the EDITORS_SEARCH variable and stick its former contents all on the for line:

DEFAULT_EDITOR="$(cat default-editor.txt)"

for editor in "${VISUAL}" "${EDITOR}" "${DEFAULT_EDITOR}" vi
do
	command -v "${editor}" 2>&1 && break
done

if ! command -v "${editor}" >/dev/null 2>&1
then
	# Print error and fail, yadda yadda
fi

# Run the editor whenever

Btw, not sure where "default-editor.txt" is supposed to live, in the current directory? Even when using dkms?

See commit ece6b5f4

Also: I'm using the line-split "if/then" and "for/do" form from the original scripts, but I also prefer the single-line versions @alkisg uses.

[M0les]

I'll probably run-through fixing whines from shellcheck -s sh *.sh as a single commit and then possibly go through some "reformatting" like making all the if/then and for/do statements live on a single line.

[alkisg]

making all the if/then and for/do statements live on a single line

Personally I prefer test xxx && yyy and test xxx || yyy to if xxx only if the whole command fits on a single line. The main goal is to make the code more readable.

Also note that:

$ if false; then echo ok; fi; echo $?
0
$ false && echo ok; echo $?
1

This means that && causes a different exit code than if, so if it's the last command that a function executes, the exit code of the function is different, and it may cause scripts to exit if they're ran under set -e:

$ bash -c 'echo 1; set -e; f() { if false; then echo ok; fi }; f; echo 2'
1
2
$ bash -c 'echo 1; set -e; f() { false && echo ok; }; f; echo 2'
1

[M0les]

making all the if/then and for/do statements live on a single line

Personally I prefer test xxx && yyy and test xxx || yyy to if xxx only if the whole command fits on a single line. The main goal is to make the code more readable.

Ah, yeah, sorry - I just meant the "if/then" part of the statement, not the code block within. So:

if ${CONDITION}; then
   # Code block

instead of:

if ${CONDITION}
then
    # Code block

Similar for for, while and until.

[M0les]

OK, I've just raised a new PR for all this stuff. It's a bit dense, so would be best looked-at as individual commit diffs.

I've given each script a one-pass test and they seem to function nominally. Given the proximity of the next release, it might be worthwhile pushing this to the following release, though.

[M0les]

Also: I'm assuming that docs/Guides/btcoex/script/btcoex_lnx.sh is a vendor off-the-shelf script/example or something, so I've not looked at it (But it's really simple syntax and while ShellCheck whines about it's errors and safety, it doesn't point-out any "bashisms" to modify.

[alkisg]

Suppose a user runs sudo sh install-driver.sh.
And in the first line, we do:

#!/bin/bash

# Re-run the script with bash if the user launched it with another shell
test -n "$BASH_VERSION" || exec "$0" "$@"

This then bypasses the problem, while allowing us to keep using bash.

Of course this idea is of interest only if bash has some extra stuff that we don't want to miss.
(my own scripts are usually compatible with dash, busybox ash and bash, but not strictly POSIX because I want to use "local" etc)

[morrownr]

@M0les @alkisg

Quick message about my status: my internet has been down since late last night. It just came back to life. It may not stay up. We had 10 inches of snow last night and it was a wet heavy snow and could be the cause of the outage.

I will catch up as I can. I just don't want you guys to think I am ignoring you.

Nick

[morrownr]

In #54 we got a bit off-topic talking about ...

We did and it is mostly my fault. I need to learn to play nice with others and keep things more orderly.

if we want to support running in non-Bash shells, is to just convert them to not use Bash-specific features (i.e. /bin/sh Bourne-shell compatibility), then converting to non-bash form would be better and more maintainable. So that's what this issue seeks to resolve.

I did some reading last night to see if I could come to an opinion as to whether non-bash would be more widely compatible and more maintainable. I also did some work with shellcheck. At this point, I don't a big advantage to using the bash specific code. On the other hand, are we fixing a problem that does not exist? My experience over the last few years is that the problem does exist but it comes up rarely. So, for me, it is a toss up with a lean toward non-bash.

Btw, not sure where "default-editor.txt" is supposed to live, in the current directory? Even when using dkms?

It only needs to live in one place... the cloned directory. It is needed by install-driver.sh and edit-options.sh. Both are generally run from the cloned directory.

Now, dkms is a different situation as dkms operates with the copied directory. Remember that dkms mandates that a copy of the directory be copied to a specific location:

cp -rf "${DRV_DIR}" /usr/src/${DRV_NAME}-${DRV_VERSION}

dkms' installation is a 4 part process: cp, add. build and install. While you can run the add, build and install steps while in the cloned directory, they actually use the copied directory but dkms does not use default-editor.txt.

I've just raised a new PR for all this stuff

I took a look. I think I will go ahead and merge so as to do some testing. We can make additional changes as needed.

Given the proximity of the next release, it might be worthwhile pushing this to the following release, though.

There really is no rush. The original goal was to release in the 1st quarter. The 23-2-2 date can be slipped or we could just open the repo to the public and continue on. This driver was picked to be the one for big upgrades to the scripts for a reason. The reason being that it has the lowest traffic of the drivers here at the site. It does not hurt that the old version is a very stable driver so people are not climbing the walls for a new version. We can just do what we think is best.

I'm assuming that docs/Guides/btcoex/script/btcoex_lnx.sh is a vendor off-the-shelf script/example or something

Almost all of the files in /docs were provided by Realtek in the original archive. As far a scripts are concerned, ignore /docs.

Nick

[morrownr]

@M0les @alkisg

Weather is still not good here but internet seems stable so I am on my dev box today. I merged the PR and plan to do some testing today. I am also planning to do one merge today with cosmetic only items. I plan to leave all of the lines you merged as they are.

One issue for discussion:

The following is the last section in install-driver.sh as the repo stands right now with your PR merged:

# if NoPrompt is not used, ask user some questions
if [ $NO_PROMPT -ne 1 ]; then
	printf "Do you want to edit the driver options file now? [y/N] "
	read -r REPLY
	echo
	case "$REPLY" in
		[yY]*) ${TEXT_EDITOR} /etc/modprobe.d/${OPTIONS_FILE} ;;
	esac

	printf "Do you want to reboot now? (recommended) [y/N] "
	read -r REPLY
	echo
	case "$REPLY" in
		[yY]*) reboot ;;
	esac
fi

I have not started testing yet I think that solution will require the use of Enter.

Here is the solution I came up with that operates exactly like the previous bash solution...

# if NoPrompt is not used, ask user some questions
if [ $NO_PROMPT -ne 1 ]
then
	printf 'Do you want to edit the driver options file now? (y/N) '
	old_stty_cfg=$(stty -g)
	stty raw -echo
	answer=$(head -c 1)
	stty "$old_stty_cfg"
	if [ "$answer" != "${answer#[Yy]}" ]
	then
		${TEXT_EDITOR} /etc/modprobe.d/${OPTIONS_FILE}
	fi
	echo

	printf 'Do you want to reboot now? (recommended) (y/N) '
	old_stty_cfg=$(stty -g)
	stty raw -echo
	answer=$(head -c 1)
	stty "$old_stty_cfg"
	if [ "$answer" != "${answer#[Yy]}" ]
	then
		reboot
	fi
	echo
fi

Now, I realize my version is long and possibly open to bugs due to variations in the versions of things that are used in the many distros so if either of you have any thoughts, please let me know. The behavior where only one key needs to be pressed is desirable in my opinion.

Nick

[alkisg]

Personally I prefer consistency in my terminal.
And most of the programs there prompt me to type things, then press Enter.
This is also a bit safer in some cases than just pressing the "y" key
(imagine a cat walking over the keyboard while the terminal was asking you if you want to erase something).
But that's my humble vote, feel free to completely ignore it.

As for the coding style, if mundane things like prompting, need many lines to be implemented, I prefer to move them into functions so that the main code is more readable.

[morrownr]

Alkis,

Consistency and safety. You made a good case. I'm allergic to cats but sometimes my birds end up on the keyboard so I understand. I'll get my hands off of Miles' code.

As for the coding style, if mundane things like prompting, need many lines to be implemented, I prefer to move them into functions so that the main code is more readable.

I agree with that statement. @Jibun-no-Kage pinged me on not having any functions back in November. I think my reply went something like this. I have no problems using functions but for these scripts, I really only want to add functions to prevent the reuse of the same code blocks but that does not really happen in these scripts. Additionally, I want a clean, straight flow so as to encourage users to be able to follow along and submit PR's with good ideas. These scripts are pretty much just one time use code within each script so functions are of limited benefit... in my humble opinion.

Miles,

Something of note: I am moving the below lines to be with the section of code that they are supporting. The constants at the top of the script are constants that can be and are in most cases used in multiple places. The following are only used in support of one section of code and I think it will help to move the lines to be with the code blocks they are supporting:

SMEM=$(LANG=C free | awk '/Mem:/ { print $2 }')
sproc=$(nproc)
DEFAULT_EDITOR="$(cat default-editor.txt)"

No code changes. Just moving location.

Nick

[morrownr]

Cosmetic cleanup merged.

I will be testing script function first and if that goes okay, I will be going over the docs as I suspect our work on the scripts is going cause changes in the README.md file as a minimum.

Cheers

[Jibun-no-Kage]

What? Did I say something? :)

[M0les]

Sorry, I'm currently in 28°C temperatures for an Australia Day break in Merimbula, so I only have my smart phone and thumb to work with for the next day or so.

I too generally prefer the "response + enter" mode of operation, but I'm not going to lose any sleep if it remains as immediate. That being said, I didn't deliberately change it, I just couldn't figure out a way to keep it the old way.

I also like the rationale for only having functions for "large-ish" duplicated code blocks (like the new response handling code would be). Generally keeping the scripts small/readable for new users is commendable.

As we're in the area of slight semantic changes, there are a couple of changes I'd consider for the future:

• Maybe not having a text editor could be a non-fatal warning, rather than a showstopper?
• The "install" script could really call out to the "edit config" script instead of having a duplicate of the code inlined. The fact it fails early if the editor is not found is a problem, though (see preceding point).

[morrownr]

I'm currently in 28°C temperatures for an Australia Day break in Merimbula...

Sigh! It is -5C here right now.

I too generally prefer the "response + enter" mode of operation, but I'm not going to lose any sleep if it remains as immediate. That being said, I didn't deliberately change it, I just couldn't figure out a way to keep it the old way.

After the discussion about cats (Alkis), dogs (Jibun) and birds (me) running around on the keyboard, your way won the day . I have been doing some testing and other than minor documentation updates. Things seem to be working fine.

Those are good ideas.

[Jibun-no-Kage]

We can make the call on what is used and how. Period. That is a core right of the author(s) of a solution. I am not saying we don't consider the impact to users by selecting a given shell, what I am saying is, you use the best shell for our solution.

You all may have noticed that I have not commented directly on which shell is used, this is in part because I am not, at least as yet, contributing to the scripting that is part of the our solution, with nods to those that have contributed. I believe that doing so at this point would just add to the confusion, positive confusion, but confusion just the same.

My suggestion is, can we please, end the debate, such as it is? I strongly suggest we stick to what we have extensively tested, which per my understand is bash based, and the logic long tested thus far. If we need to change in the future, lets do it based on the issues and resulting user experience that suggests any such possible change.

At this late stage, the suggestion that we significantly change anything, I would not recommend. This is my extensive QA/QC experience over my IT career suggesting this course.

Will understand if the team deems otherwise to my recommendation above, of course, but I felt the above needed to be said, as due diligence. I have seen at times excellent solutions get buried in analysis paralysis, simply because a line in the sand was not drawn.

No solution is ever perfect, no solution is ever truly complete. And maybe most key, is that no solution is truly universal, at least not yet, in my experience.

[Jibun-no-Kage]

Oh, a question, since I have two Dachshunds, do I actually get 3 votes, on whatever we vote on?

[morrownr]

@Jibun-no-Kage

At this late stage, the suggestion that we significantly change anything, I would not recommend. This is my extensive QA/QC experience over my IT career suggesting this course.

I understand and agree. While I was offline for a day, I worked on the scripts to see what changes would be required. With the exception of 2 items, all shellcheck recommended items were extremely low risk in that they simply did not make any logic changes. It was mostly format changes to increase reliability and compatibility with far more distros out there. Shellcheck is pretty slick. I wish I had known about it before.

One item that required a change was to how we handle the questions at the end of the scripts that have questions. Function was changed from 1 button to 1 button plus Enter due to the differences in the read command. Very low risk.

Overall, the changes that Miles included in the PR were very similar or exactly as mine. Miles' PR looked like he was rewriting an encyclopedia (scary stuff) but most of his PR was actually coding style differences. Miles likes having more than one command on a line in some situations and I, well, not so much but I prefer consistency so I am good with what he did.

In the end, I merged Miles' PR because I felt it would increase overall compatibility and was worth the very minor risk. I have been testing with sh and bash and am not seeing any problems. The scripts have been solid and still are solid, largely because of the extensive testing that you conducted early on. Will we find something here in the last week due to the last minute changes? Maybe but I doubt it in this case. If we do, I take my Crow medium well.

Since we started back in November, a lot of work has gone into the scripts and docs. I think the original goal of reducing problem reports by >50% may happen. A lot of additions are working. Last week I was testing something for a company that I do consulting for. I burned a clean sd card with RasPiOS. I booted and immediately decided to test this driver instead of doing what I was supposed to be doing. The script stopped and told me that the header files were not properly installed. I was able to immediately see what the problem was and before I reported this ongoing problem, I did an update and upgrade and the problem was fixed. Without the code that Alkis provided, I would have had dkms spewing crap that really did not help solve problems.

If you get a chance, do a couple of tests and see if you see any problems.

Nick

[Jibun-no-Kage]

Cool. Works for me. Just thought it was good to call a sanity check. :) Ah, I guess I don't get 3 votes then? LOL

[morrownr]

@M0les

I like your ideas as I said and here are a couple of my ideas for the future:

• Since around kernel 5.3, Linux has supported compressed drivers. How about we add that capability?

• We were unable to get to the bottom on one cause of many issues. The regdb, country code mess. I gathered a lot of information and finally decided it would be hard to put code together given our currently level of knowledge, One of the main problems seems to be distros doing different things and I am still fuzzy on exactly how things should be.

I'd really like to keep this group of collaborators together in this repo so as to continue working various issues. Things don't really have to change much just because the repo is opened to the public.

Here is something that some of you might have an adapter for:

Alkis and I created a repo recently to work on the rtl8852bu driver. Alkis mentioned that we could invite others to test and help so I eventually want to put a message in issues in USB-WiFi but one can only do what they have time to do and it has not happened yet. Any of your are welcome to join. I can put you in the repo immediately.

Do not run out and buy an adapter based on a rtl8852bu chipset if you don't have or can borrow one. If you want or need WiFi 6 capability, get an adapter based on the mt7921au chipset. The driver is in-kernel, stable and supports 6 GHz. I haven't seen anything about 6 GHz from Realtek.

Nick

[morrownr]

Cool. Works for me. Just thought it was good to call a sanity check. :) Ah, I guess I don't get 3 votes then? LOL

Look. I have two birds so that cancels out your two dogs. LOL

[alkisg]

Miles likes having more than one command on a line in some situations and I, well, not so much but I prefer consistency

In vscode, I've installed the shell-format extension and I automatically format my shell scripts by pressing Ctrl+Shift+I.

Internally, this calls the shfmt program, so anyone that doesn't like vscode can run that one instead.

Initially I disagreed with 5% of its formatting choices (agreeing with 95% is veeery good!), but as you say, for consistency with the rest of the world, I decided to give in an use its defaults.
(P.S. shfmt doesn't like multiple commands in one line)

[morrownr]

In vscode, I've installed the shell-format extension and I automatically format my shell scripts by pressing Ctrl+Shift+I.

I am going to take a look. Appreciate the info.

(P.S. shfmt doesn't like multiple commands in one line)

Shhh... don't tell Miles.

I realize I get out of step with modern styles at times... I learned programming with FORTRAN. There was no such thing as 2 or more commands on a line. Heck, there were no monitors or mice...

[Jibun-no-Kage]

The only problem with auto-formatting... is I hate the assumptions others make about code style/layout. I like MY format. LOL!