Skip to content

with_bootloader issues #286

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

Closed
sleemanj opened this issue Aug 29, 2018 · 7 comments
Closed

with_bootloader issues #286

sleemanj opened this issue Aug 29, 2018 · 7 comments
Assignees
Labels
conclusion: resolved Issue was resolved help wanted Assistance from the community is especially welcome topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project

Comments

@sleemanj
Copy link

sleemanj commented Aug 29, 2018

The "with_bootloader" export which happens here...

https://github.com/arduino/arduino-builder/blob/29cf4c8b1473b4d0ba429c67ee497e2e061bd2fc/merge_sketch_with_bootloader.go

has some issues.

  1. There is no output or even logging that it happened, so we can't for example see which bootloader it picked easily, how much it threw away, and that sort of thing.

  2. The attached zip contains a compiled sketch .hex, the bootloader which it (in theory) selected, and the merged hex. The merged hex however contains the sketch hex, and two lines only from the bootloader hex - you will see clearly it can't be right because the bootloader is 9.9k, and the "merged" file which should contain both the sketch and bootloader is only 640 bytes.

@sleemanj
Copy link
Author

The aforementioned zip I forgot to attach...

bad_hex.zip

@sleemanj
Copy link
Author

sleemanj commented Aug 29, 2018

I believe that this issue (2 above) is down to the bootloader including a (blank) configuration section as the last few bytes (these are used by the bootloader to store an OSCAL value when "tuned"), I expect that the "blank line" code in the .go file linked above is running through and finding this and thinking everything before is sketch code and dumping it.

It looks like bootloader.noblink was maybe supposed to be a way to address that, but looking at the code above extractActualBootloader is run over the bootloader regardless if it came from bootloader.noblink or bootloader.file

Unless noblink is supposed to mean something other than I imagine it to (a quick search couldn't really turn up anything educational, it hardly seems to be used), that being "a raw bootloader which can be merged as is", then that would be the way to fix this problem perhaps, fix merge_sketch_with_bootloader.go so that if using bootloader.noblink it does not extractActualBootloader

There is also another problem to be added to the list however

  1. If a bootloader is not suitable for merging, there is no way to indicate this so that a useless result is not produced. The primary example here is for "Virtual Boot Partition" bootloaders on chips that do not have boot reset vectors and instead put an appropriate jump at vector 0 and re-write any incoming upload to shift the uploaded code's vector 0 somewhere else which the bootloader will jump to. This type of bootloader can not be merged successfully without prior knowledge of how the bootloader works, the merging result will probably run the sketch just fine but the bootloader would never get run will probably run the bootloader just fine but never run the sketch (since the reset vector in the bootloader would appear later in the merged hex and blow away the sketch's one) so it wouldn't be useful as a merge, and might be confusing for people since there would be no warning or error indicating the issue.

Perhaps noblink would better be named merge and could include the possibility to be set to none to prevent the bootloader merged output from being generated for those, ie mychip.bootloader.merge=mergable_bootloader.hex or mychip.bootloader.merge=none

sleemanj added a commit to sleemanj/optiboot that referenced this issue Aug 30, 2018
…help with merged exports.

Virtual boot partition bootloaders will not merge correctly, but the standard ones should (because the notuner_ variants do not include the OSCAL configuration bytes).

See arduino/arduino-builder#286
@facchinm
Copy link
Member

Hi @sleemanj ,
the "merge bootloader" functionality has always been very naive.
Just a bit of background: the first (and only) board to need it was the Yun, since it used a combined binary for remote uploading. The original code was VERY specific to the 32u4 bootloader so it failed badly with other targets; the noblink/blink naming means that one contains the bootloader only while the other contains the bootloader+a Blink sketch .
I experimented a bit with the merge functionality (#192) but never had time to clean it up.

As I see now, the best format to merge is Intel Hex since it also contains information on the flash offset. A (very needed) patch could grab bootloader and sketch in hex format and merge them in a smart way (by understanding the sections, not by copy/paste).

A second patch could reuse recipe.objcopy.bin.pattern to generate the corresponding bin file for targets that need it.

Any help in doing this would be very appreciated 😉 !

@facchinm facchinm added the help wanted Assistance from the community is especially welcome label Aug 30, 2018
@facchinm facchinm self-assigned this Sep 17, 2018
@facchinm
Copy link
Member

@sleemanj would you mind testing #295 an see if it solves your issue?

@tushar-semwal
Copy link

anybody on this issue? I am having similar problems with SAMD boards. I am trying to upload the with_bootloader hex file using J-Link flasher, however throws up "not able to parse" error.

@facchinm
Copy link
Member

#295 should be rebased on arduino-cli and eventually merged after some testing. I'd love to have time for this but maybe someone from the community can pick it up.

@cmaglie
Copy link
Member

cmaglie commented Jun 12, 2020

fixed by arduino/arduino-cli#744

@cmaglie cmaglie closed this as completed Jun 12, 2020
@rsora rsora added the type: enhancement Proposed improvement label Sep 22, 2021
@per1234 per1234 added topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project conclusion: resolved Issue was resolved and removed type: enhancement Proposed improvement labels Oct 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conclusion: resolved Issue was resolved help wanted Assistance from the community is especially welcome topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants