-
Notifications
You must be signed in to change notification settings - Fork 145
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Some bugfixes for the tools/test-avrdude script #1891
Conversation
de3e3af
to
29133dc
Compare
Ah. Just noticed that properly failing if |
0b46554
to
6995b3f
Compare
@ndim Great catch! I ever so slightly prefer the following variant: diff --git a/tools/test-avrdude b/tools/test-avrdude
index 233f5c86..8b030058 100755
--- a/tools/test-avrdude
+++ b/tools/test-avrdude
@@ -158,8 +158,8 @@ tmpfile=$(mktemp "$tmp/$progname.tmp.XXXXXX")
resfile=$(mktemp "$tmp/$progname.res.XXXXXX")
trap "rm -f $status $logfile $outfile $tmpfile $resfile" EXIT
-echo -n "Testing '$avrdude_bin -v'..."
-$avrdude_bin -v 2>&1 | grep '[vV]ersion' | cut -f2- -d: | sed s/Version/version/ > "$outfile"
+echo -n "Testing $avrdude_bin "
+$avrdude_bin -v 2>&1 | grep ersion | sed s/"^.* [Vv]"ersion/version/ | head -1 > "$outfile"
if grep version "$outfile" > /dev/null 2>&1; then
cat "$outfile"
else |
8227f62
to
b14a856
Compare
There's something bothering me about test-avrdude. For example, when I try to pass Or would it be better if dry-run didn't exit with an error for unknown -x? |
950a11a
to
b1d561c
Compare
Done, AFAICT. |
@ndim Thanks for fixing the [vV]ersion bug and for making the script run on older bash versions. Looks great! Just checking whether you need the first line of the output
My preference would be a slightly crisper output:
@askn37 @MCUdude Yes, currently cannot pass @ndim Both points would be solved by the following patch diff --git a/tools/test-avrdude b/tools/test-avrdude
index 192bee63..5e9e6f42 100755
--- a/tools/test-avrdude
+++ b/tools/test-avrdude
@@ -149,12 +149,7 @@ fi
arraylength=${#pgm_and_target[@]}
-echo -n "Testing executable type for '$avrdude_bin'..."
-if type "$avrdude_bin" >/dev/null 2>&1; then
- echo -n " "
- type "$avrdude_bin"
-else
- echo
+if ! type "$avrdude_bin" >/dev/null 2>&1; then
echo "$progname: cannot execute $avrdude_bin"
exit 1
fi
@@ -167,13 +162,12 @@ tmpfile=$(mktemp "$tmp/$progname.tmp.XXXXXX")
resfile=$(mktemp "$tmp/$progname.res.XXXXXX")
trap "rm -f $status $logfile $outfile $tmpfile $resfile" EXIT
-echo -n "Testing $avrdude_bin version..."
-$avrdude_bin -v 2>&1 | grep '[vV]ersion' | sed 's/^.* [Vv]ersion //' | head -n1 > "$outfile"
+echo -n "Testing "$(type -p "$avrdude_bin")
+$avrdude_bin -v 2>&1 | grep -i version | sed 's/^.* version//i' | head -n1 > "$outfile"
if test -s "$outfile"; then
- echo -n " "
cat "$outfile"
else
- echo " error"
+ echo ": error obtaining version using $avrdude_bin -v"
$avrdude_bin -v
exit 1
fi
@@ -273,9 +267,9 @@ for (( p=0; p<$arraylength; p++ )); do
avrdude=($avrdude_bin -l $logfile $avrdude_conf -qq ${pgm_and_target[$p]})
# Get flash and EEPROM size in bytes and make sure the numbers are in dec form
- flash_size=$(${avrdude[@]} -cdryrun -T 'part -m' 2>/dev/null | grep flash | awk '{print $2}')
+ flash_size=$("$avrdude_bin" -c dryrun -p $part -T 'part -m' 2>/dev/null | grep flash | awk '{print $2}')
bench_flwr_size=$((flash_size/6)) # Approximate(!) size of file holes_rjmp_loops_${flash_size}B.hex
- ee_size=$(${avrdude[@]} -cdryrun -T 'part -m' 2>/dev/null | grep eeprom | awk '{print $2}')
+ ee_size=$("$avrdude_bin" -c dryrun -p $part -T 'part -m' 2>/dev/null | grep eeprom | awk '{print $2}')
bench_eewr_size=$((ee_size/6)) # Approximate(!) size of file holes_pack_my_box_${ee_size}B.hex
if [[ -z "$flash_size" ]]; then How should we do this? Do you want to apply this patch? Should I push a commit onto your branch? |
The output of the version information in "avrdude -v" has changed between avrdude 7.3 and now (commit cf0822b): avrdude: Version 7.3 Avrdude version 7.3-20240814 (250a663a) This means that the old grep call looking for "Version" cannot find anything any more, and therefore needs to be changed. This was not discovered as the test-avrdude script neglected to abort if no version information is found.
Use the uninstalled avrdude executable and the avrdude.conf config file which have just been built for the dry-run test with tools/test-avrdude. Note that the parameter for the config file is a bit unexpected: -c "-C path/to/avrdude.conf"
Macos ships a quite old version of bash (3.x) which does not have associative arrays (declare -A) yet. As the test-avrdude script only uses one associative array for a static mapping of one character strings to one word strings, this can be easily replaced by a shell function containing a "case" statement. Before this fix: 2024-08-15T15:39:59.9596380Z Prepare "-cdryrun -pm2560" and press 'enter' or 'space' to continue. Press any other key to skip 2024-08-15T15:40:00.5796130Z ✅ 0.155 s: flash raw format -T/-U write/verify cola-vending-machine.raw 2024-08-15T15:40:00.7006170Z ✅ 0.111 s: flash extended address and hole test 2024-08-15T15:40:00.7057180Z ./tools/test-avrdude: line 322: declare: -A: invalid option 2024-08-15T15:40:00.7057750Z declare: usage: declare [-afFirtx] [-p] [name[=value] ...] 2024-08-15T15:40:00.9218810Z ✅ 0.212 s: flash writing R numbers 2024-08-15T15:40:01.1793950Z ✅ 0.240 s: flash reading and verifying R numbers 2024-08-15T15:40:01.3661060Z ✅ 0.173 s: flash writing R numbers 2024-08-15T15:40:01.6317540Z ✅ 0.254 s: flash reading and verifying R numbers 2024-08-15T15:40:01.8235780Z ✅ 0.182 s: flash writing R numbers 2024-08-15T15:40:02.0724260Z ✅ 0.239 s: flash reading and verifying R numbers 2024-08-15T15:40:02.2713120Z ✅ 0.188 s: flash writing R numbers 2024-08-15T15:40:02.4497490Z ✅ 0.162 s: flash reading and verifying R numbers 2024-08-15T15:40:02.5897960Z ✅ 0.128 s: flash writing R numbers 2024-08-15T15:40:02.8667350Z ✅ 0.263 s: flash reading and verifying R numbers 2024-08-15T15:40:03.0635260Z ✅ 0.181 s: flash writing srec format 2024-08-15T15:40:03.2152560Z ✅ 0.142 s: flash reading and verifying srec format file After this fix: 2024-08-15T17:23:24.5161820Z Prepare "-cdryrun -pm2560" and press 'enter' or 'space' to continue. Press any other key to skip 2024-08-15T17:23:25.1088990Z ✅ 0.162 s: flash raw format -T/-U write/verify cola-vending-machine.raw 2024-08-15T17:23:25.2742090Z ✅ 0.157 s: flash extended address and hole test 2024-08-15T17:23:25.4795650Z ✅ 0.196 s: flash writing binary numbers 2024-08-15T17:23:25.6721130Z ✅ 0.180 s: flash reading and verifying binary numbers 2024-08-15T17:23:25.8423490Z ✅ 0.161 s: flash writing octal numbers 2024-08-15T17:23:26.0901770Z ✅ 0.235 s: flash reading and verifying octal numbers 2024-08-15T17:23:26.2308860Z ✅ 0.127 s: flash writing decimal numbers 2024-08-15T17:23:26.4667010Z ✅ 0.228 s: flash reading and verifying decimal numbers 2024-08-15T17:23:26.6574670Z ✅ 0.180 s: flash writing hexadecimal numbers 2024-08-15T17:23:26.8651240Z ✅ 0.200 s: flash reading and verifying hexadecimal numbers 2024-08-15T17:23:27.0446640Z ✅ 0.168 s: flash writing R numbers 2024-08-15T17:23:27.3074230Z ✅ 0.250 s: flash reading and verifying R numbers 2024-08-15T17:23:27.4962750Z ✅ 0.179 s: flash writing srec format 2024-08-15T17:23:27.6594570Z ✅ 0.150 s: flash reading and verifying srec format file
a8d98d1
to
2ec3263
Compare
Condense the information about the avrdude binary executable and its version from two output lines into a single output line. Slight adaptation of code by Stefan Rueger from avrdudes#1891 (comment)
2ec3263
to
a2906da
Compare
Determine the flash and ee memory sizes without invoking the full ${avrdude[@]} command with logging etc. Slight adaptation of code by Stefan Rueger from avrdudes#1891 (comment)
@stefanrueger I have taken up that patch and reworked it a bit, splitting it into two commits. The first commit condenses two output lines into one output line:
The second commit deals with
Applied, with the changes described above. |
@ndim Well spotted re the This should now be ready for merging. |
While working on #1888, I have found and fixed a few issues with
tools/test-avrdude
:Since commit cf0822b, the
avrdude -v
version line looks different, and therefore thetest-avrdude
code which deals with the version does not work any more. Impact: The output looks likeinstead of
The fix is easy and just make grep look for both "version" and "Version".
If
avrdude -v
does not run, the remaining tests intools/test-avrdude
will not run either, so aborting with a useful error message showing the complete output ofavrdude -v
makes sense.As it is not clear yet which
avrdude
executabletools/test-avrdude
is actually running, it makes sense to... well not so sure about that any more. But having the complete output oftype $avrdude_bin
has certainly been helpful when trying to run the wrongavrdude
executable, and that is what that commit does.tools/test-avrdude
uses bash associative arrays. Associative arrays are a feature introduced with bash 4.0, so the bash version shipped with MacOS (3.x) does not support associative arrays. I have replaced the associative array with a shell function.@stefanrueger Not sure whether you want to merge any of this before 8.0. I would suggest at least cherrypicking 29133dc.