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

[feature request] It would be nice to have the Arduino IDE add a #define for the configured flash size #6806

Closed
sprior opened this issue Nov 19, 2019 · 9 comments · Fixed by #6690

Comments

@sprior
Copy link

sprior commented Nov 19, 2019

In cases where specific code depends on the existence of FS configured flash, it would be nice to have a #define available to prevent compilation/upload if the Flash config is misconfigured, especially in an OTA situation where this could prevent the device from booting.

For example if the code depends on saving device settings to SPIFFS and therefore booting is impossible with no FS configured, it would be great to be able to code a safety measure like:
#ifndef FS1MB
#error The Flash Config must be set to 1M FS
#endif

This would prevent an OTA upload where the device couldn't possibly boot afterwards and save a lot of hassle.

@devyte
Copy link
Collaborator

devyte commented Nov 19, 2019

Look at FS_PHYS_SIZE and _FS_start.
Closing due to not compliant.

@devyte devyte closed this as completed Nov 19, 2019
@sprior
Copy link
Author

sprior commented Nov 19, 2019

I just set my Arduino IDE to verbose compilation output and neither string PHYS or FS_ appears in it, so I don't see what you're talking about.

@devyte
Copy link
Collaborator

devyte commented Nov 19, 2019

What does the IDE compilation output have to do with anything?

https://github.com/esp8266/Arduino/blob/a389a995fb12459819e33970ec80695f1eaecc58/cores/esp8266/flash_hal.h

You want to code a safety measure for the FS size, you should be able to use the FS_PHYS_SIZE #define in the above link for that.

BTW, there is a dark gray box in the top left of the repo webpage where you can do code searches within the repo. Just copy paste the symbols I referenced above and you get the files where they're used.

@sprior
Copy link
Author

sprior commented Nov 19, 2019

Unless I'm mistaken it appears to me that the macros defined in flash_hal.h will only allow you to check the FS size at runtime, not cause the compilation to fail at compile time since they refer to extern "C" locations.

What I'm trying to address in this issue is to allow a safety measure to be made at compile time which will prevent successful code compliation and upload to the device if the required FS size is not met.

If you've already uploaded the code to the device before you discover the error at runtime you very possibly can't recover from this kind of misconfiguration. And it's very easy to misconfigure this because it gets reset every time you change the board variation.

@devyte
Copy link
Collaborator

devyte commented Nov 19, 2019

The symbols are not defined at runtime. As I said, you can search the repo, or clone and investigate the code to figure out things like that.
You're thinking of preprocessor time, while I'm pointing you towards compile time. Most likely what you want is a static assert with the content of the #define, or an ASSERT in a linker script.

Please be aware that this is an issue tracker, meant for tracking issues in the core, and not meant for general "how do I" type discussions. as explained in the issue template which you saw (and ignored) when you opened this.

@d-a-v
Copy link
Collaborator

d-a-v commented Nov 19, 2019

It is true that there is no way to know the FS-size at compile time.
We can (cheaply) add symbols matching the IDE configuration menu.

@sprior Have a look to #6690 and if it doesn't better answer to your needs, please add [feature request] to this issue's title.

@sprior sprior changed the title It would be nice to have the Arduino IDE add a #define for the configured flash size [feature request] It would be nice to have the Arduino IDE add a #define for the configured flash size Nov 19, 2019
@sprior
Copy link
Author

sprior commented Nov 19, 2019

Thanks d-a-v, #6690 does look very interesting, but I think that my request would co-exist with that. The reason is that the other issue is adding a new but non-default option to the menu. The motivation for my request is that when you change board types the other menu items reset to their defaults and in the case of FS config you may need to ensure you remember to change it back or else you'll brick the device on upload (even more annoying when it's an OTA, guess how I know this).

So my request could dovetail nicely with #6690 if you could add an #if structure that checks to make sure you either set the new option for #6690 OR set the FS size manually, otherwise an #error directive would tell you to change the config before compile/upload would be allowed.

@devyte
Copy link
Collaborator

devyte commented Nov 19, 2019

Reopening after internal discussion

@sprior
Copy link
Author

sprior commented Nov 20, 2019

Thanks for the change, looks good.

@d-a-v d-a-v added this to the 3.0.0 milestone Nov 9, 2020
@d-a-v d-a-v modified the milestones: 3.0.0, 3.0.1 Mar 31, 2021
@d-a-v d-a-v modified the milestones: 3.0.1, 3.1 Jun 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants