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

trim DFU bootloader, enable BANGLE1 SPI flashing #2449

Merged
merged 3 commits into from
Jan 15, 2024

Conversation

fanoush
Copy link
Contributor

@fanoush fanoush commented Jan 15, 2024

disable noncritical errata at startup (will get applied when main application starts)
disable unused protobuf decoders
disable gcc startup code
enable SPI flashing for BANGLEJS 1 (may not work for now, fits with 8 bytes free)

- disable noncritical errata
- disable unused protobuf decoders
fanoush referenced this pull request Jan 15, 2024
@fanoush
Copy link
Contributor Author

fanoush commented Jan 15, 2024

I did test this with other boards that it does not break DFU itself and bootloader still works as before.
I did not test with Bangle 1 at all as I don't have Bangle 1 board with SWD available.

@gfwilliams
Copy link
Member

This looks great - thanks!

I'll wait until I have some time to test before merging though - given it's such a pain to reflash Bangle.js 1 I really want to make sure this doesn't break the bootloader!

While this is really neat, I'm honestly not sure if adding it to the Bangle.js 1 is something I want to do... I'm not aware of anyone that asked for this feature, and realistically on Bangle.js 1 for most people the result of a failed firmware update is going to be a bricked watch...

It just feels like the risk of this being a support nightmare might be higher than any potential benefit to Bangle.js 1 users.

Obviously if you have devices you're using this with then I'm all for keeping your changes in the repo - I'm just not sure if we should enable it for Bangle.js 1 by default.

@fanoush
Copy link
Contributor Author

fanoush commented Jan 15, 2024

I'm honestly not sure if adding it to the Bangle.js 1 is something I want to do...

Yes, I understand the device is old and this may generate some additional support/documentation effort which may be not worth it for you. So yes, I think you can keep it in and still do not enable that SPI flashing feature for BANGLE 1, just have some more space in bootloader for something else.

I'm not aware of anyone that asked for this feature

Well, the reason is same as with Bangle 2 - Nordic DFU is not the most stable thing on all platforms and generates some pain for users and this new method avoids that completely. But yes I looked at https://github.com/espruino/BangleApps/blob/master/apps/fwupdate/custom.html and for now it is very bangle 2 specific so would need some effort.

Obviously if you have devices you're using this with then I'm all for keeping your changes in the repo

Thanks, yes, I plan to enable this SPI flashing on some 52832 based watches and to see what happens I will try to add display code at least to some of them so I will probably hit same limits as Bangle1

So I can rename PR to remove "enable BANGLE1 SPI flashing" and drop the BANGLEJS board file patch.
What about the rest? makefile patch, gcc startup code

  • BOOTLOADER_DEFINES for passing something only to bootloader can be useful - even some current flags like the insecure dfu could be set with this prefix so it is not defined for main application. CFLAGS,LDFLAGS,ASFLAGS are for more special cases but won't hurt having it there
  • removing gcc startup code saves ~420 bytes, if you would want to go for it for all nrf52 boards, it could move to common makefile
  • the EXTREME flag saves about 180 bytes and is mostly harmless

Or it could be just there without enabling it for anything right now. Except BANGLE 1 I don't know where the bootloader is so tight. And even there with reduced vector table there is some space to grow even now.

@gfwilliams
Copy link
Member

Thanks! I'll just add a change to comment out the lines in the python file, with a comment about what they do if you put them in - the rest is fine, and I'll merge :)

Comment out Bangle.js 1 with explanation
@gfwilliams gfwilliams merged commit 57ae5ee into espruino:master Jan 15, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants