Fix memory size defines

[matthijskooijman]

This fixes a number of macros in avr/io.h that used to hardcode memory sizes, but now base on the actual STM32 header files. In particular, this affects the EEPROM size E2END, which also affects EEPROM.length().

[GrumpyOldPizza]

Not quite. I am using hardcoded values for EEPROM, because 2k of the EEPROM is reserved for system purposes (in that case LoRaWAN session data).

The value for the RAM size was hardcoded originally to avoid pulling in stm32l0xx.h. Some of the code still pulls that in via Arduino.h, so it's kind of a "who cares" at that point of time.

Ideally this should be going into some "wiring_xyz.h" file so that another port can properly set that.

[matthijskooijman]

Not quite. I am using hardcoded values for EEPROM, because 2k of the EEPROM is reserved for system purposes

Ah, I see. Though just hardcodedly reserving 2k of EEPROM is probably not the best approach in general, since:

• It sounds like a lot more than really needed
• Sketches might not actually use the LoRaWAN library shipped with this core

I'm working on a board that uses this core but indeed does not use the LoRaWAN library, but I want to store some write-once
configuration data at the end of EEPROM (with pretty much the same goal: To stay out of the way of any EEPROM usage by the sketch).

Unfortunately, the standard EEPROM library has no meaningful API for this, i.e. no way to say "this bit of EEPROM is reserved for use by libraries" or something like that. This could be useful, but should probably be done in the default Arduino cores first in order to standardize (or maybe there is some third-party library that does something like this alread that could be adopted or recommended?).

Also, not all of the chips in the L0 series have 6k of EEPROM, some don't even have 2k it seems. Maybe all currently supported boards do have 6k, but it would of course be nicer to make this stuff as general as possible (to allow defining boards with smaller chips too).

Maybe a compromise could be to define this as a board option? i.e. -DEEPROM_RESERVED_TOP=2048 in the variant fil and/or boards.txt and then autodetect the EEPROM size and subtract that value? Then the default boards could keep the default 2048, and my custom board could just use 0 (or maybe another value that is suitable for my usecase). Maybe a REAL_E2END could be exposed to with the original value? This even leaves open the option of adding a board menu to choose whether or not to reserve EEPROM)?

An alternative could be to use __has_include(...) to detect whether or not the LoRaWAN library is actually being used. This is a builtin macro that checks whether a header is available in the include path. Since the Arduino IDE only puts libraries in the include path that are actually being #included (and __has_include does trigger library inclusion). There is a small caveat where __has_include prevents gcc from emitting proper errors and breaking IDE include detection, but that can be worked around by not testing for an actual meaningful .h file, but just a dummy (see e.g. hideakitai/ArxTypeTraits#1 (comment)).

In particular, this would mean adding a libraries/LoRaWAN/src/stm32l0_lorawan_used (or whatever) file, and then doing something like:

#define REAL_E2END (...calculation...)
if __has_include(<stm32l0_lorawan_used>)
#define E2END (REAL_E2END - 2048)
#else
#define E2END REAL_E2END
#endif

I'm not quite sure what the best approach is here, though.

The value for the RAM size was hardcoded originally to avoid pulling in stm32l0xx.h.

But given there are also L0 chips that have smaller RAM, autodetecting this would probably be more correct?

Ideally this should be going into some "wiring_xyz.h" file so that another port can properly set that.

What do you mean by "another port" here? I.e. to allow sharing code between STM32l0 and l4?

[GrumpyOldPizza]

You are right about different L0 chips. Got to fix that. L052 has 8k SRAM and 2k EEPROM (no LoRaWAN support there).

LoRaWAN needs 2k. I do not want to go into all the gory details, but yes it needs that.

What might be the best way to handle this is to have some STM32L0_CONFIG_xyz style defines to override the EEPROM size. Just needed to change Arduino.h then to include "variant.h" before "avr/eeprom.h"

But given there are also L0 chips that have smaller RAM, autodetecting this would probably be more correct?

SRAM probably yes, EEPROM no.

What do you mean by "another port" here? I.e. to allow sharing code between STM32l0 and l4?

Perhaps ... who knows. At least the STM32WB/WL port share the same core/arduino and libraries as a starting point.

[matthijskooijman]

What might be the best way to handle this is to have some STM32L0_CONFIG_xyz style defines to override the EEPROM size. Just needed to change Arduino.h then to include "variant.h" before "avr/eeprom.h"

I'm not sure if board-dependent reservation is the perfect solution, but it would work for me for now (I can just disable the reservation in my board file then).

Note that I think avr/eeprom.h should actively include variant.h, rather than relying on Arduino.h to do so. Consider the case where a library includes avr/eeprom.h before (or without) Arduino.h, then there's a chance of it getting the wrong value. Testing this, I found that directly including variant.h does not work (since variant.h does not include all the other headers it needs, so it relies on being included by Arduino.h after any other stuff it needs), but including Arduino.h from io.h does work (which creates a loop, but that is cleanly broken by the include guards and works as long as Arduino.h includes everything io.h needs before including io.h itself.

SRAM probably yes, EEPROM no.

Wouldn't it be cleaner to just autodetect the EEPROM size (and set e.g. REAL_E2END and then subtract a variant.h-defined amount from that)? Then it is more clear what is going on, creating a new variant from an existing one does not require any changes when the EEPROM size changes (provided the EEPROM is still >= 2k) and then maybe the LoRaWAN library can be simplified to use REAL_E2END rather than doing its own autodetection as it does now?

I've implemented the combination of the above and replaced the commits in this PR. See the commit message for additional details.

Open questions:

• I now did #define STM32L0_CONFIG_EEPROM_RESERVED 2048 for all the boards that (I think) have a LoRa transceiver, or would you prefer this for all of them with sufficient EEPROM?
• For other boards, I just left out the define entirely, or would you prefer defining it explicitly to 0?
• If the STM32L0_CONFIG_EEPROM_RESERVED is not defined, it is assumed to be 0. Or would it make sense to default to 2048 for externally defined boards that rely on the old behavior?
• I also wanted to change the LoRaWAN library to do something like #define EEPROM_OFFSET_START (REAL_E2END + 1 - 1024), but I'm not quite sure what the intention of the existing define is. It seems to round the autodetected size up to a multiple of 1024, which is unneeded when the EEPROM size is a multiple of 1024 and actually results in overflowing the EEPROM when it is not a multiple of 1024 AFAICS). Also, 2k is reserved, but from a quick look at these defines, only 1k is used. Maybe these are the gory details you mentioned. If this change is applied, then LoRaWAN.cpp should probably also have an assert that checks that the STM32L0_CONFIG_EEPROM_RESERVED is actually big enough and users trying to use the library on other cores and/or cores with too little EEPROM will get a nicer error message.

[GrumpyOldPizza]

I am not happy with messing with the size there. I see I need to fix L052, but nobody complianed yet.

For L072, do you have a real problem with only 4k EEPROM, or is it a matter of cleanness of the code ?

My issue there is that memory is managed in general by the platform code. Say some boot code ... you could say, yes but I don't need it ... Or the section layout in the linker file with some reserved holes. Or the fact that the I2C code is using ISR/DMA, where you could write blocking code that needs less. Or the fact that 4 or the 5 RTC_BKUP registers are used up internally. The L072 platform has 2k EEPROM reserved for system purposes. L052 will not reserve anything at the moment, but might move to reserve 512 bytes, or 1024 bytes going forward.

[matthijskooijman]

I am not happy with messing with the size there. I see I need to fix L052, but nobody complianed yet.

How is setting the correct size messing?

For L072, do you have a real problem with only 4k EEPROM, or is it a matter of cleanness of the code ?

I need to put my write-once settings (i.e. id and keys) at the end of EEPROM (both because I want to keep the EEPROM start free for sketches to use, and because I need to coordinate this with my board manufacturer that also wants to store some tracing code and is settling on an "end of EEPROM" convention).

I can probably bypass the EEPROM.length() / E2END and do the detection myself (and then just call the STM32 EEPROM functiosn or eeprom_read_block or so directly), but even then I'm losing up to 2k of EEPROM for no good reason. I don't have any specific need for that memory right now, but this is intented as a generic development platform, so I'm not quite prepared to make this part of the EEPROM inaccessible just because that was easier.

[GrumpyOldPizza]

Mind pining me directly via grumpyoldpizza@gmail.com.

…

On Wed, Nov 4, 2020 at 10:43 AM Matthijs Kooijman ***@***.***> wrote: I am not happy with messing with the size there. I see I need to fix L052, but nobody complianed yet. How is setting the correct size messing? For L072, do you have a real problem with only 4k EEPROM, or is it a matter of cleanness of the code ? I need to put my write-once settings (i.e. id and keys) at the end of EEPROM (both because I want to keep the EEPROM start free for sketches to use, and because I need to coordinate this with my board manufacturer that also wants to store some tracing code and is settling on an "end of EEPROM" convention). I can probably bypass the EEPROM.length() / E2END and do the detection myself (and then just call the STM32 EEPROM functiosn or eeprom_read_block or so directly), but even then I'm losing up to 2k of EEPROM for no good reason. I don't have any specific need for that memory right now, but this is intented as a generic development platform, so I'm not quite prepared to make this part of the EEPROM inaccessible just because that was easier. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#166 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABXBA7AANZXO6UN2JTDEO4LSOGHFJANCNFSM4TKDCJSQ> .