Comments (26)
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?
from 8821cu-20210916.
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.
from 8821cu-20210916.
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.
from 8821cu-20210916.
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
from 8821cu-20210916.
making all the if/then and for/do statements live on a single line
Personally I prefer
test xxx && yyy
andtest xxx || yyy
toif 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
.
from 8821cu-20210916.
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.
from 8821cu-20210916.
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.
from 8821cu-20210916.
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)
from 8821cu-20210916.
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
from 8821cu-20210916.
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
from 8821cu-20210916.
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
from 8821cu-20210916.
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.
from 8821cu-20210916.
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
from 8821cu-20210916.
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
from 8821cu-20210916.
What? Did I say something? :)
from 8821cu-20210916.
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).
from 8821cu-20210916.
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.
from 8821cu-20210916.
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.
from 8821cu-20210916.
Oh, a question, since I have two Dachshunds, do I actually get 3 votes, on whatever we vote on?
from 8821cu-20210916.
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
from 8821cu-20210916.
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
from 8821cu-20210916.
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
from 8821cu-20210916.
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
from 8821cu-20210916.
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)
from 8821cu-20210916.
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...
from 8821cu-20210916.
The only problem with auto-formatting... is I hate the assumptions others make about code style/layout. I like MY format. LOL!
from 8821cu-20210916.
Related Issues (20)
- Nvidia Jetson Nano HOT 3
- No traffic for 4-5 seconds every minute HOT 2
- [Issue]: Hang on linux-6.6.5.arch1-1 HOT 4
- Very poor performance HOT 9
- (this is a kernel bug) not working on 6.6.5-arch1-1 HOT 4
- Good work but not yet there. HOT 1
- Variable behaves unexpectedly in dkms.conf HOT 9
- bpf error HOT 1
- Can be ported to FreeBSD? HOT 1
- Adding new Device ID HOT 4
- (solved) Not working on Mint 21.3 HOT 7
- Mention that a driver appeared in kernel 6.2+ HOT 3
- (solved) Install on Fedora 39 HOT 8
- rtl8821cu Cant connect to available Wifi networks with my proximity.
- rtl8821cu drivers installing issue
- Information Request: Best method to determine driver installed? Or git update is available before doing actual git pull? HOT 8
- Difference between native kernel driver vs. 8821cu-20210916? More networks found with Nick variant? HOT 3
- kernel >=6.8 will not work HOT 12
- Creating a VIF hangs HOT 4
- Help with CDROM mode in RTL8188GU chipset HOT 7
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from 8821cu-20210916.