Try to run ChipErase on UPDI even if return code is -67

[umbynos]

More details in arduino/ArduinoCore-megaavr#33 (comment)

Co-authored-by: Martino Facchin [email protected]

Related conversation with @MCUdude umbynos@b6d21d1#commitcomment-73387352

[MCUdude]

Looks good to me! This has been incorporated in the Arduino flavored Avrdude version for a couple of years now and has not caused any issues AFAIK.

[MCUdude]

For reference, 67 is documented in pyedbglib as

# avr8protocol.py
AVR8_FAILURE_CRC_FAILURE = 0x43  # CRC operation caused an error

[MCUdude]

@umbynos slightly off-topic, but will Arduino's Avrdude fork be available on downloads.arduino.cc when you've finished working on your 7.0 fork?

[umbynos]

@MCUdude actually is already available on the download servers:

• http://downloads.arduino.cc/tools/avrdude_7.0-arduino.1_Linux_32bit.tar.gz
• http://downloads.arduino.cc/tools/avrdude_7.0-arduino.1_Linux_64bit.tar.gz
• http://downloads.arduino.cc/tools/avrdude_7.0-arduino.1_Linux_ARM64.tar.gz
• http://downloads.arduino.cc/tools/avrdude_7.0-arduino.1_Linux_ARMv6.tar.gz
• http://downloads.arduino.cc/tools/avrdude_7.0-arduino.1_macOS_64bit.tar.gz
• http://downloads.arduino.cc/tools/avrdude_7.0-arduino.1_Windows_32bit.tar.gz

We still miss a test package_index.json file and a core to test this out. All the work I did is available here, as I've announced here

[MCUdude]

Thanks, @umbynos! I managed to get my boards manager file working with the links you provided.

After a bit of testing, I'm a bit bummed out that your dev branch lacks a few features. Most noticeably, I managed to get non-standard baud rates working with macOS through PR #898. However, this fix is not present in your branch. Is this something that's happened by accident, or is it left out on purpose?

[umbynos]

My dev branch was there only for testing purposes. Well, #898 should be included, since the release workflow should check out tag v7.0 and then apply the patches present here. The commit 99f191a should be included in v7.0 tag. Are you sure that that commit is not included? The patches will be removed from there if they will be upstreamed.

[MCUdude]

@umbynos sorry, I take it all back! It turned out to be some flakey hardware on my side

[MCUdude]

@dl8dtl or @mariusgreuel can we merge this as it is?

[dl8dtl]

@dl8dtl or @mariusgreuel can we merge this as it is?

Well, I don't particularly like those "magic numbers" – not only in the patch but also in the existing code.
Instead of adding a comment on top, why not use the respective names in the first place (and add a new name for code 67 as well)?
Now, that we are going to touch that piece of code anyway, it would be a good time now to correct all that.

Apart from that, it's fine.

[MCUdude]

@dl8dtl AFAIK the reason why the magic number 68 was used in the first place is because it's defined in jtag3_private.h, and is not available in main.c.

We could either include jtag3_private.h in main.c, or add another comment.

      if (rc != 0) {
        // -67 == -(0x47) == -(RSP3_FAIL_CRC_FAILURE)
        // -68 == -(0x44) == -(RSP3_FAIL_OCD_LOCKED)
        if ((rc == -67 || rc == -68) && (p->flags & AVRPART_HAS_UPDI) && (attempt < 1)) {
``

[dl8dtl]

The we'd better not pass that number straight into main() anyway.
Remember, the goal is that the code ought to be usable as a library, so all return values need to be documented (ultimately, yes, I know, we're quite a bit away from that).
So it would make sense to add some kind of soft failure error code to the public header file, and return that one in case either RSP3_FAIL_CRC_FAILURE or RSP3_FAIL_OCD_LOCKED is seen in the lower (library) layers.

Yes, it's a bit more of work, but a lot cleaner then.

[MCUdude]

So it would make sense to add some kind of soft failure error code to the public header file, and return that one in case either RSP3_FAIL_CRC_FAILURE or RSP3_FAIL_OCD_LOCKED is seen in the lower (library) layers.

I'm not sure I follow you... Which public header; jtag.h?

[dl8dtl]

Which public header; jtag.h?

libavrdude.h ought to document everything that is needed to describe the API to the underlying functions.

[dl8dtl]

https://github.com/dl8dtl/avrdude/tree/jtag3_retcode

That's my proposal.

I can turn that into a PR if there's interest, but I don't think it makes sense to try merging it with the current PR.

[MCUdude]

@dl8dtl even though I haven't tested your code, I agree that the idea is good. The code looks fine as well 🙂

[MCUdude]

@dl8dtl would be great if you could submit your work as a PR 👍

[dl8dtl]

@dl8dtl would be great if you could submit your work as a PR +1

#996

Would be great if in particular those who suffered from the original issue could test that.

[dl8dtl]

Superseded by #996