Skip to content
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

Enable stdin file verification and display correct number of bytes written/verified #1051

Closed
wants to merge 48 commits into from

Conversation

stefanrueger
Copy link
Collaborator

This PR fixes Issues #1044, #441 and #1005, and should help for PR #1036

Counting the number of bytes written to a memory and/or verified is not trivial owing to potential holes in the input file and to potential trailing 0xff bytes in flash memory that are not written per default (but see -A). The new function memstats(), which is best called just after an input file has been read into mem->buf/mem->tags, computes the right number of bytes written and allows easy computation of the number of bytes verified.

This PR also changes the strategy for the default verification after writing to a chip memory, so that the input file only needs reading once thus enabling successful verification of stdin input files.

Other, minor changes:

  • Improving the grammar of AVRDUDE output, eg, 1 byte written instead of 1 bytes written
  • Better description of the input file structure in terms of its sections, the interval it spans, the number of pages, the number of padding bytes in pages, and the number of actually cut off trailing 0xff bytes for flash
  • Printing or instead of - in the -U routines
  • Option -V no longer needs to be specified before option -U in order to work

As an aside this PR also provides useful helper functions for printing plural(), inname(), outname() and interval() all of which return strings fit for printing.

Examples using blink-mega2560+lext-test.hex and sketch-ending-in-ff.hex:

$ avrdude -qp ATmega2560 -c ... -U blink-mega2560+lext-test.hex

avrdude: AVR device initialized and ready to accept instructions
avrdude: Device signature = 0x1e9801 (probably m2560)
avrdude: NOTE: "flash" memory has been specified, an erase cycle will be performed
         To disable this feature, specify the -D option.
avrdude: erasing chip
avrdude: input file blink-mega2560+lext-test.hex auto detected as Intel Hex
avrdude: reading input file blink-mega2560+lext-test.hex
         with 1346 bytes in 4 sections within [0, 0x3106d]
         using 7 pages and 446 pad bytes
avrdude: writing 1346 bytes flash ...
avrdude: 1346 bytes of flash written
avrdude: verifying flash memory against blink-mega2560+lext-test.hex
avrdude: 1346 bytes of flash verified

avrdude done.  Thank you.
$ avrdude -qp ATmega328P -c ... -U sketch-ending-in-ff.hex

avrdude: AVR device initialized and ready to accept instructions
avrdude: Device signature = 0x1e950f (probably m328p)
avrdude: NOTE: "flash" memory has been specified, an erase cycle will be performed
         To disable this feature, specify the -D option.
avrdude: erasing chip
avrdude: input file sketch-ending-in-ff.hex auto detected as Intel Hex
avrdude: reading input file sketch-ending-in-ff.hex
         with 2160 bytes in 1 section within [0, 0x888]
         using 17 pages and 16 pad bytes, cutting off 25 trailing 0xff bytes
avrdude: writing 2160 bytes flash ...
avrdude: 2160 bytes of flash written
avrdude: verifying flash memory against sketch-ending-in-ff.hex
avrdude: 2185 bytes of flash verified

avrdude done.  Thank you.
$ echo "Hello, world..." | avrdude -qp ATmega328P -c ... -U eeprom:w:-:r

avrdude: AVR device initialized and ready to accept instructions
avrdude: Device signature = 0x1e950f (probably m328p)
avrdude: reading input file <stdin>
          with 16 bytes in 1 section within [0, 0xf]
         using 4 pages and 0 pad bytes
avrdude: writing 16 bytes eeprom ...
avrdude: 16 bytes of eeprom written
avrdude: verifying eeprom memory against <stdin>
avrdude: 16 bytes of eeprom verified

avrdude done.  Thank you.

…rified

Counting the number of bytes written to a memory and/or verified is not
trivial owing to potential holes in the input file and to potential trailing
0xff bytes in flash memory that are not written per default (but see -A). The
new function memstats(), which is best called just after an input file has
been read into mem->buf/mem->tags, computes the right number of bytes written
and allows easy computation of the number of bytes verified.

This commit also changes the strategy for the default verification after
writing to a chip memory, so that the input file only needs reading once thus
enabling successful verification of stdin input files.

Other, minor changes:
 - Improving the grammar of AVRDUDE output, eg, 1 byte written instead of
   1 bytes written
 - Better description of the input file structure in terms of its sections,
   the interval it spans, the number  of pages, the number of padding bytes
   in pages, and the number of actually cut off trailing 0xff bytes for flash
 - Printing <stdin> or <stdout> instead of - in the -U routines

As an aside this commit also provides useful helper functions for printing
plural(), inname(), outname() and interval() all of which return strings fit
for printing.

Examples:

$ avrdude -qp ATmega2560 -c ... -U blink-mega2560+lext-test.hex

avrdude: AVR device initialized and ready to accept instructions
avrdude: Device signature = 0x1e9801 (probably m2560)
avrdude: NOTE: "flash" memory has been specified, an erase cycle will be performed
         To disable this feature, specify the -D option.
avrdude: erasing chip
avrdude: input file blink-mega2560+lext-test.hex auto detected as Intel Hex
avrdude: reading input file blink-mega2560+lext-test.hex
         with 1346 bytes in 4 sections within [0, 0x3106d]
         using 7 pages and 446 pad bytes
avrdude: writing 1346 bytes flash ...
avrdude: 1346 bytes of flash written
avrdude: verifying flash memory against blink-mega2560+lext-test.hex
avrdude: 1346 bytes of flash verified

avrdude done.  Thank you.

$ avrdude -qp ATmega328P -c ... -U sketch-ending-in-ff.hex

avrdude: AVR device initialized and ready to accept instructions
avrdude: Device signature = 0x1e950f (probably m328p)
avrdude: NOTE: "flash" memory has been specified, an erase cycle will be performed
         To disable this feature, specify the -D option.
avrdude: erasing chip
avrdude: input file sketch-ending-in-ff.hex auto detected as Intel Hex
avrdude: reading input file sketch-ending-in-ff.hex
         with 2160 bytes in 1 section within [0, 0x888]
         using 17 pages and 16 pad bytes, cutting off 25 trailing 0xff bytes
avrdude: writing 2160 bytes flash ...
avrdude: 2160 bytes of flash written
avrdude: verifying flash memory against sketch-ending-in-ff.hex
avrdude: 2185 bytes of flash verified

avrdude done.  Thank you.

$ echo "Hello, world..." | avrdude -qp ATmega328P -c ... -U eeprom:w:-:r

avrdude: AVR device initialized and ready to accept instructions
avrdude: Device signature = 0x1e950f (probably m328p)
avrdude: reading input file <stdin>
         with 16 bytes in 1 section within [0, 0xf]
         using 4 pages and 0 pad bytes
avrdude: writing 16 bytes eeprom ...
avrdude: 16 bytes of eeprom written
avrdude: verifying eeprom memory against <stdin>
avrdude: 16 bytes of eeprom verified

avrdude done.  Thank you.
@stefanrueger stefanrueger added bug Something isn't working enhancement New feature or request labels Aug 1, 2022
@MCUdude
Copy link
Collaborator

MCUdude commented Aug 1, 2022

Thank you Stefan! I can confirm that #1005 and #1044 is resolved by this PR

Without this PR:

Arduino sketch with bootloader in the same hex file:

$ ./avrdude -cusbasp -patmega328p -Uflash:w:/var/folders/6l/ypg6qbw172v1s4vtt6g990tw0000gn/T/arduino_build_563573/Blink.ino.with_bootloader.hex:i

avrdude: AVR device initialized and ready to accept instructions

Reading | ################################################## | 100% 0.00s

avrdude: Device signature = 0x1e950f (probably m328p)
avrdude: NOTE: "flash" memory has been specified, an erase cycle will be performed
         To disable this feature, specify the -D option.
avrdude: erasing chip
avrdude: reading input file "/var/folders/6l/ypg6qbw172v1s4vtt6g990tw0000gn/T/arduino_build_563573/Blink.ino.with_bootloader.hex"
avrdude: writing flash (32768 bytes):

Writing | ################################################## | 100% 0.31s

avrdude: 32768 bytes of flash written
avrdude: verifying flash memory against /var/folders/6l/ypg6qbw172v1s4vtt6g990tw0000gn/T/arduino_build_563573/Blink.ino.with_bootloader.hex:

Reading | ################################################## | 100% 0.18s

avrdude: 32768 bytes of flash verified

avrdude done.  Thank you.

Write to EEPROM from stdin:

$ yes "The quick brown fox jumps over the lazy dog"| head -2 | ./avrdude -cusbasp -p atmega328p -U eeprom:w:-:r

avrdude: AVR device initialized and ready to accept instructions

Reading | ################################################## | 100% 0.00s

avrdude: Device signature = 0x1e950f (probably m328p)
avrdude: reading input file <stdin>
         with 88 bytes in 1 section within [0, 0x57]
         using 22 pages and 0 pad bytes
avrdude: writing 88 bytes eeprom ...

Writing | ################################################## | 100% 0.92s

avrdude: 88 bytes of eeprom written
avrdude: verifying eeprom memory against <stdin>

Reading | ################################################## | 100% 0.03s

avrdude: 88 bytes of eeprom verified

avrdude done.  Thank you.


With this PR:

Arduino sketch with bootloader in the same hex file:

$ ./avrdude -cusbasp -patmega328p -Uflash:w:/var/folders/6l/ypg6qbw172v1s4vtt6g990tw0000gn/T/arduino_build_563573/Blink.ino.with_bootloader.hex:i

avrdude: AVR device initialized and ready to accept instructions

Reading | ################################################## | 100% 0.00s

avrdude: Device signature = 0x1e950f (probably m328p)
avrdude: NOTE: "flash" memory has been specified, an erase cycle will be performed
         To disable this feature, specify the -D option.
avrdude: erasing chip
avrdude: reading input file /var/folders/6l/ypg6qbw172v1s4vtt6g990tw0000gn/T/arduino_build_563573/Blink.ino.with_bootloader.hex
         with 1514 bytes in 3 sections within [0, 0x7fff]
         using 13 pages and 150 pad bytes
avrdude: writing 1514 bytes flash ...

Writing | ################################################## | 100% 0.30s

avrdude: 1514 bytes of flash written
avrdude: verifying flash memory against /var/folders/6l/ypg6qbw172v1s4vtt6g990tw0000gn/T/arduino_build_563573/Blink.ino.with_bootloader.hex

Reading | ################################################## | 100% 0.20s

avrdude: 1514 bytes of flash verified

avrdude done.  Thank you.

Write to EEPROM from stdin:

$ yes "The quick brown fox jumps over the lazy dog"| head -2 | ./avrdude -cusbasp -p atmega328p -U eeprom:w:-:r

avrdude: AVR device initialized and ready to accept instructions

Reading | ################################################## | 100% 0.00s

avrdude: Device signature = 0x1e950f (probably m328p)
avrdude: reading input file "<stdin>"
avrdude: writing eeprom (88 bytes):

Writing | ################################################## | 100% 0.91s

avrdude: 88 bytes of eeprom written
avrdude: verifying eeprom memory against -:

Reading | ################################################## | 100% 0.00s

avrdude: 0 bytes of eeprom verified

avrdude done.  Thank you.

@MCUdude
Copy link
Collaborator

MCUdude commented Aug 1, 2022

Another thing to point out is that previously, a memory with size 0 would be ignored, but now it doesn't continue.

This is how the "fake" efuse defined for ATmega8 in avrdude.conf. This trick is currently being used by Arduino IDE.

    memory "efuse"
        size            = 0;
      ;

Without this PR:

$ ./avrdude -C /Users/hans/Documents/Arduino/hardware/MiniCore/avr/avrdude.conf -c usbasp -p atmega8 -Uefuse:w:0xff:m -Ulfuse:w:0xff:m

avrdude: AVR device initialized and ready to accept instructions

Reading | ################################################## | 100% 0.00s

avrdude: Device signature = 0x1e9307 (probably m8)
avrdude: reading input file "0xff"
avrdude: writing efuse (0 bytes):

Writing | ################################################## | 100% 0.00s

avrdude: 0 bytes of efuse written
avrdude: verifying efuse memory against 0xff:

Reading | ################################################## | 100% 0.00s

avrdude: 0 bytes of efuse verified
avrdude: reading input file "0xff"
avrdude: writing lfuse (1 bytes):

Writing | ################################################## | 100% 0.00s

avrdude: 1 bytes of lfuse written
avrdude: verifying lfuse memory against 0xff:

Reading | ################################################## | 100% 0.00s

avrdude: 1 bytes of lfuse verified

avrdude done.  Thank you.

With this PR:

./avrdude -C /Users/hans/Documents/Arduino/hardware/MiniCore/avr/avrdude.conf -c usbasp -p atmega8 -Uefuse:w:0xff:m -Ulfuse:w:0xff:m

avrdude: AVR device initialized and ready to accept instructions

Reading | ################################################## | 100% 0.00s

avrdude: Device signature = 0x1e9307 (probably m8)
avrdude: reading input file 0xff
avrdude: ATmega8 efuse size 0 or page size 1 not set

avrdude done.  Thank you.

@stefanrueger
Copy link
Collaborator Author

a memory with size 0 would be ignored

@MCUdude Great catch! I have relaxed the sanity checks, so memory size zero is OK again

$ avrdude -qp m8 -F -c ... -U efuse:w:0xff:m && echo OK

avrdude: AVR device initialized and ready to accept instructions
avrdude: Device signature = 0x1e950f (probably m328p)
avrdude: Expected signature for ATmega8 is 1E 93 07
avrdude: reading input file 0xff
         with 0 bytes in 0 sections within [0, -1]
avrdude: writing 0 bytes efuse ...
avrdude: 0 bytes of efuse written
avrdude: verifying efuse memory against 0xff
avrdude: 0 bytes of efuse verified

avrdude done.  Thank you.

OK

File reads are limited to the memory they are destined for, which explains the 0 bytes read (of the immediate file with one byte).

@stefanrueger stefanrueger changed the title Enable stdin file verification and correct number of bytes written/verified Enable stdin file verification and display correct number of bytes written/verified Aug 1, 2022
@mcuee
Copy link
Collaborator

mcuee commented Aug 2, 2022

I can confirm that #441 and #1005 are now fixed.

PS C:\work\avr\avrdude_test\avrdude_bin> .\avrdude_pr1051 -c usbasp -qp m2560 
-U .\blink-mega2560+lext-test.hex -V

avrdude_pr1051.exe: AVR device initialized and ready to accept instructions
avrdude_pr1051.exe: Device signature = 0x1e9801 (probably m2560)
avrdude_pr1051.exe: NOTE: "flash" memory has been specified, an erase cycle will be performed
                    To disable this feature, specify the -D option.
avrdude_pr1051.exe: erasing chip
avrdude_pr1051.exe: input file .\blink-mega2560+lext-test.hex auto detected as Intel Hex
avrdude_pr1051.exe: reading input file .\blink-mega2560+lext-test.hex for flash
                    with 1346 bytes in 4 sections within [0, 0x3106d]
                    using 7 pages and 446 pad bytes
avrdude_pr1051.exe: writing 1346 bytes flash ...
avrdude_pr1051.exe: 1346 bytes of flash written

avrdude_pr1051.exe done.  Thank you.

@mcuee
Copy link
Collaborator

mcuee commented Aug 2, 2022

#1044 is also okay now.

$ yes "The quick brown fox jumps over the lazy dog" | ./avrdude -c usbasp -p m2560 -U eeprom:w:-:r

avrdude.exe: AVR device initialized and ready to accept instructions

Reading | ################################################## | 100% 0.01s

avrdude.exe: Device signature = 0x1e9801 (probably m2560)
avrdude.exe: reading input file <stdin> for eeprom
             with 4096 bytes in 1 section within [0, 0xfff]
             using 512 pages and 0 pad bytes
avrdude.exe: writing 4096 bytes eeprom ...

Writing | ################################################## | 100% 42.54s

avrdude.exe: 4096 bytes of eeprom written
avrdude.exe: verifying eeprom memory against <stdin>

Reading | ################################################## | 100% 1.15s

avrdude.exe: 4096 bytes of eeprom verified

avrdude.exe done.  Thank you.

ffontaine and others added 15 commits August 2, 2022 20:02
Fix the following build failure without a C++ compiler:

CMake Error at CMakeLists.txt:24 (project):
  No CMAKE_CXX_COMPILER could be found.

  Tell CMake where to find the compiler by setting either the environment
  variable "CXX" or the CMake cache entry CMAKE_CXX_COMPILER to the full path
  to the compiler, or to the compiler name if it is in the PATH.

Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
…errors

The new file type I is essentially Intel HEX that, on download, inserts
comments next to data records with the resolved effective address and an
ASCII dump of that same record. On upload the `I` format is permissive
with respect to check sum errors, eg, after manipulated an Intel HEX file
for debugging.
…on -p \*

  -p \*/c  check address bits in SPI commands
  -p \*/d  description of core part features
  -p \*/o  opcodes for SPI programming parts and memories
  -p \*/s  show avrdude.conf entries of parts
  -p \*/ss show full avrdude.conf entry as tab separated table
  -p \*/w  wd_... constants for ISP parts
  -p \*/\* all of the above except -p \*/s
  -p \*    same as -p\*/\*
When an SPI command has a lone 'a' the initialisation now is as would be
expected by all commands that take an address. Atmel's opcodes for SPI
programming are consistent in this respect. This commit makes specifying
the bit number in avrdude.conf optional. Instead of

 read_lo = "0 0 1 0 0 0 0 0  0 0 a13 a12 a11 a10 a9 a8  a7 a6 a5 a4 a3 a2 a1 a0  o o o o o o o o";

one can now use

 read_lo = "0 0 1 0 0 0 0 0  0 0 a a a a a a  a a a a a a a a  o o o o o o o o";
stefanrueger and others added 22 commits August 2, 2022 20:02
…ndaries

The function avr_set_addr_mem(AVRMEM *mem, int opnum, unsigned char *cmd,
unsigned long addr) is meant to replace avr_set_addr(OPCODE *op, unsigned
char *cmd, unsigned long addr) in future.

avr_set_addr_mem() has more information about the context of the task in that
it knows the memory size, memory page size, whether or not the memory is a
flash memory (which gets words addressees supplied) and, crucially, knows
which SPI operation it is meant to compute the address bits for.

avr_set_addr_mem() first computes the interval of bit numbers that must be
supplied for the SPI command to stand a chance to work. The function only
sets those address bits that are needed. Once all avr_set_addr() function
calls have been replaced by avr_set_addr_mem(), the SPI commands that need an
address can afford to declare in avrdude.conf all 16 address bits in the
middle two bytes of the SPI command. This over-declaration will be corrected
during runtime by avr_set_addr_mem(). One consequence of this is that parts
can inherit smaller or larger memories from parents without the need to use
different SPI codes in avrdude.conf. Another consequence is that
avr_set_addr_mem() can, and does, tell the caller whether vital address bits
were not declared in the SPI opcode. During parsing of avrdude.conf this
might be utilised to generate a corresponding warning. This will uncover
problematic SPI codes in avrdude.conf that in the past went undetected.
…no as ISP

For paged read/write early AVRDUDE implementations of the STK500 v1 protocol
communicated a word address (below a_div=2) or byte address (a_div=1) based
on the following code irrespective of which memories were used:

  if(m->op[AVR_OP_LOADPAGE_LO] || m->op[AVR_OP_READ_LO])
    a_div = 2;
  else
    a_div = 1;

This turned out to be a bug: it really should have been a_div=2 for flash and
a_div=1 for eeprom. At the time presumably no one noted because Atmel was at
the cusp of replacing their FW 1.x with FW 2 (and the STK500 v2 protocol).

It seems that the world (optiboot, Arduino as ISP, ...) has compensated for
the bug by assuming AVRDUDE sends *all* eeprom addresses as word addresses.
Actually these programmers overcompensated for the bug because for six out of
the 146 known SPI programmable parts with eeprom and page size > 1, AVRDUDE
would still send the eeprom addresses as byte addresses (ATmega8 ATmega8A
ATmega64 ATmega64A ATmega128 ATmega128A) owing to above code.

It makes no sense to correct the bug now seeing that virtually no one uses
the old 2005 STK 500 v1 firmware. This commit now follows optiboot, Arduino
as ISP and other projects, and simply sends all addresses for paged read or
write as word addresses. There are no longer (little known) exceptions for
ATmega8 et al that surprised some optiboot etc users.
@MCUdude
Copy link
Collaborator

MCUdude commented Aug 2, 2022

@stefanrueger I think something went wrong when updating your fork. You just added 44 commits to this PR, and all of them have already been merged into main.

@stefanrueger
Copy link
Collaborator Author

@MCUdude One of my goals in July was to understand git rebase upstream/main main followed by a local rebase of my branch followed by an elegant git push upstream without using a clickety-dickety web interface ... and I failed (again): git reports diverging branches, refuses to do what I think needs doing and suggests a solution that works but is inelegant: Technically, the PR is still what need doing (I can tell by looking at the changes that this PR effects), but has the disadvantage of a non-linear history (as you spotted redoing commits that already were done). I can abandon this PR and copy over the changed files for a new attempt (OK because I did not have an elaborate commit history for this PR) or we tolerate the non-linear history. Thoughts? I am OK either way.

@MCUdude
Copy link
Collaborator

MCUdude commented Aug 2, 2022

It's fairly easy to just drop the commits that doesn't belong in this PR.

  • First, do a backup, just in case.
  • Run git log --oneline and copy the 2nd short hash in the list (should be 84fa3ee I think)
  • Run git rebase -i 84fa3ee
  • In the text file that shows up, replace the first-word pick with drop on every commit that you want to remove from this branch.
  • When you're done, save the file and force-push the changes by using git push -f.

@stefanrueger
Copy link
Collaborator Author

@MCUdude Thanks for tips. I can confirm this has worked, and the source stayed the same. git log shows me that I am where I want to be (I think!): a rebased branch with the additions at the end. However, now the branch on my fork is 48 commits ahead, 57 commits behind avrdudes:main. So, it looks like if I was to merge this we'd be at some weird commit history (again). The git guru in my company reckons there is a simple solution: delete my repositories, remove my account, recursively delete my home directory, uninstall the OS, raze my disks, give notice, apply for a new identity, relocate under a witness protection scheme, and start again :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants