search

SignalK / SensESP

Universal Signal K sensor framework for the ESP32 platform
https://signalk.org/SensESP/
Apache License 2.0
151 stars 81 forks source link

Memory usage of SensESP #239

Closed KEGustafsson closed 6 months ago

KEGustafsson commented 3 years ago

Memory usage of ESP82866 (mayby also ESP32) has been increased from what it was earlier (e.g. summer time) compared today's usage rate. This is now resulting situations that older codes, which were running OK earlier, are not running correctly due to lack of memory. I have seen that code can run (data coming to SK), but WebUI config is not working or device has become unstable (random reboots, multiple access requests, etc...)

Test code that I used resulted following available memory amounts: 5-Nov: 1.6-2.2k (latest commit) 17-Sep: 5.2-5.5k (75a0c5e6e4f094e5666f3077d19030fb1432ff71) Aug/Sep: 11.9-12.1k (can't remember commit) => In use on the boat

Aug/Sep SensESP is only one, which open WebUI config correctly. Newer versions with low amount of memory will hang the device.

I'm not really sure how to replicate old situation again because some of the code is pulled online while compiling and therefore replication of Aug/Sep setup is bit unclear how to do it. In any case, free memory amount has been decreasing with SensESP updates and compatibility with old codes are not any more 100% sure.

JohnySeven commented 3 years ago

@KEGustafsson you can specify commit in lib_deps and when you delete .pio folder it will checkout that specific commit from history. So if you comment out SensESP lib and following line instead you can use specific commit and test it:

https://github.com/SignalK/SensESP#bcd0686b270da4ed2589f53bfd588433a67516a0

Jan

KEGustafsson commented 3 years ago

@JohnySeven Thanks! I'll try to replicate old setups with commit added.

mairas commented 3 years ago

I did a bit of investigation on the topic. I was building a minimal project file (just having the system sensors enabled, nothing else) on different commits on esp8266. Here're some results (date, git hash, and typical free memory figure):

2020-09-17 75a0c5e: 19632
2020-09-29 89c787e: 19216
2020-10-15 75f723d: 19104
2020-10-18 5e4e30d: 16440
2020-10-18 e150633: 16544
2020-10-23 cb549cc: 16056
2020-11-06 d1a1485: 16544

In this timeframe the worst offender seemed to be the magnetometer code. I hadn't realized that any global variables get linked in, whether they're used or not, although that kinda makes sense if you think of it. And that code defines all of its hardware sensors as globals, and the objects are really big.

I'd be tempted to remove the magnetometer stuff altogether for now. It could be wrapped into its own library and hosted separately and only pulled in if actually needed.

Also, it would be a good idea to go through the whole codebase and look for possible offenders.

BjarneBitscrambler commented 3 years ago

I agree: including the orientation code uses more flash and RAM than building the project without it. I have two suggestions - one short term workaround, and one longer term fix:

Details

I did some testing this morning on images compiled using the different filter algorithms available. The choice of fusion algorithm (filter) has a big effect. The results were: Codebase Flash (bytes) Freemem (bytes)
Adafruit_NXP_Sensorfusion filter 1 382 633 / 1 343 889 215 980
Mahony filter 1 375 413 / 1 343 889 218 400

the two values for flash size were with / without the actual orientation sensor instantiated in main.cpp. The freemem was recorded without the orientation sensor (i.e. just the basic SensESP uptime, freemem, etc sensors).

The sensor fusion rewrite is using NXP's version 7 algorithm (as opposed to the version 4.,2 in the Adafruit/NXP AHRS library that SensESP currently uses). I think it's going to perform better, and it will definitely not have as many outside dependencies as including the AHRS library does.

KEGustafsson commented 3 years ago

Can we exclude NXP library from 8266 builds? This is not even targeted to be used in this platform and issues is in the 8266. Can there be separate libraries for 8266 an 32?

mairas commented 3 years ago

I'd argue that nonchalantly using several KB of memory for no good reason is a problem on ESP32 as well. Separating functionality based on architecture is possible but problematic for documentation, test builds and general maintenance.

@BjarneBitscrambler, could you make a PR for your suggested short-term solution? Or alternatively, the whole Orientation9DOF sensor could be split into a library of its own.

joelkoz commented 3 years ago

Hey Guys! Long time since I've been here, but I've started poking around again since I'll be starting up another project this weekend, and I've seen I've missed out on a lot of fun. In any event...

One possible approach to be looked at is using controlled text via the #ifdef directive around the offending sensor source files. For example, the code could be written in such a way that "if you want to use a particular sensor, you need to define a symbol like SENSESP_SENSOR_ORIENTATION9DOF in your platform.ini file". The entire .h and .cpp files could be wrapped in #ifdef/#endif so sensor specific code would appear as an empty file unless it is explicitly requested via the symbolic definition. That solves 1/2 the problem. The other half would be solved by activating the linker's "dead code elimination" features to not link in unused code. A quick Google search shows gcc supports such a thing via a few directives. See this post: https://stackoverflow.com/questions/6687630/how-to-remove-unused-c-c-symbols-with-gcc-and-ld

I've never been a fan of "kitchen sink" code sets (that is code that includes all possible features whether or not they are used). I am a little surprised that sensor specific code is included in the SensESP repo (outside of the "examples" tree) as SensESP was originally more of a framework than an end user application. I would think specific sensor sets would be in other libraries that build on top of SensESP. Thus, one approach is to divide the repo in sensesp-core, then add sensesp-onewire, sensesp-adafruit, sensesp-gauges, etc. as separate libraries. Those libraries can be coarse grade (i.e. all onewire type sensors in a single library), or fine grained (one library per sensor) That complicates new project creation, however. Complicated project configuration can be alleviated with scaffolding tools such as Yoeman.

A third approach is to move all sensor specific code to sub-directories under the examples. We'd create a "onewire temperature sensor" example, which would be a complete project. That keeps the references to sensor specific code (and dependent libraries in the platform.ini file) out of the main source tree.

BjarneBitscrambler commented 3 years ago

I'll open an PR to prevent the filter global from being instantiated, saving approx 2500 bytes according to my tests above. Give me a couple days to sort that out.

There may be other globals being created as part of the AHRS library - I'll poke around to see what affects the build and runtime memory usage. I suspect parts of the library that get compiled but not pulled in because they are neither used nor global won't make a difference.

The rewritten sensor fusion library shouldn't cause the same memory issues and there will be a lot fewer 3rd party library dependencies. I reckon it'll probably be ready in about 2 months.

mairas commented 3 years ago

@joelkoz thanks for the tip on the gcc options. I have to test them. I had assumed the linker would omit unused code segments by default but apparently that's not the case either!

There was a bit of discussion (or at least the case of me ranting about it) on the Slack channel about the current kitchen-sink approach. I agree with you; it doesn't work in the long term, and the specific hardware code should be in separate libraries. I'd simply create a template repo for any new sensor libraries and split the existing libraries into fine-grained repos. I don't think project setup is a huge issue with PlatformIO: you just list the libraries you need in the ini file and PIO does the rest.

One issue would be library locations: for the existing code we could probably request new repos within the SignalK project, but I think new ones should be hosted in their author's namespace. All of this leads to a discoverability problem but I think we could just maintain a list of libraries within the SensESP documentation.

ba58smith commented 3 years ago

RE: new libraries being hosted in the author's namespace: what if they one day decide to simply delete their namespace? I think that may have happened with Fritzing, and now, no one seems to be able to locate the source code for it. Could that happen with a SensESP library?

mairas commented 3 years ago

I tried adding -fdata-sections -ffunction-sections -Wl,--gc-sections to the platformio.ini build options. Linker gave some warnings about redeclaration of memory regions and the device wouldn't boot. So, unfortunately, we didn't get a free lunch.

mairas commented 3 years ago

RE: new libraries being hosted in the author's namespace: what if they one day decide to simply delete their namespace? I think that may have happened with Fritzing, and now, no one seems to be able to locate the source code for it. Could that happen with a SensESP library?

That's a risk we have to live with. I could get fed up and delete the ReactESP repo. (Wasn't going to, but now that you gave me ideas... ;-)) Paul Stoffgren might delete his OneWire library, leaving a whole lot of projects stranded. Etc etc. The only remedy is to have enough people clone the important repos so that their contents can be pushed to a new online repo and have the project links updated.

JohnySeven commented 3 years ago

@mairas I'm glad you are thinking about this as we discussed it last time. I'm willing to give any help needed to make this happen.

joelkoz commented 3 years ago

How about we create a single separate public repo in the SignalK space that holds all the individual sensors/applications - something like "SensESP-projects". Each sub-directory would be a complete project, and the source tree for that sub-directory would hold the sensor specific code. That keeps the library references needed by the sensors in the project specific platformio.ini file, eliminating kitchen sink linking yet still keeping it simple for end users: an end user would then simply have to clone a sub-directory to start a new project.

joelkoz commented 3 years ago

Another idea that may be more simple than a separate repo: create a sensors sub-directory underneath the examples directory. Underneath examples/sensors, create sub-directories for each set of sensor source files that require the same libraries (e.g. put all of the one wire sensors in examples/sensors/onewire). Move all source files out of src/sensors except the sensor.cpp and .h file. Remove any sensor specific library dependencies out of platformio.ini, and put them in a README.md file in each examples/sensors sub-directory. This README.md would tell the developer which dependencies they need to add to the platformio.ini file to use the sensor specific code.

Doing it this way eliminates the kitchen-sink linking problem, which is only going to get worse over time as new sensor specific code is added. Applications using the SensESP core would have app specific sensor source code in their individual project directories outside of the SensESP source tree. The app developer need only copy the appropriate examples/sensors/XXX source files to their local dev src directory. They would then be free to make any app specific tweaks to the sensor source code and not worry about having those be overwritten when they get an update of the library.

BjarneBitscrambler commented 3 years ago

From what I've observed during some hours of testing different build configurations, I don't believe the presence of unused libraries is causing the RAM shortage. I believe some other structural changes to the base SensESP code, or changes to a non-sensor-related library, or changes to the compiler settings, accounts for the increased RAM use compared to earlier. On my system, as long as a sensor is not instantiated in main.cpp, I saw no difference in the size of the executable and freemem as reported in SignalK, whether or not the sensor libraries are included in the build.

Some Results

To test memory footprint with/without libraries I cloned the current SensESP project and compiled it with different modifications. The changes I made were selected from:

Before each build I performed a clean; after each build I recorded the RAM and Flash size reported by the compiler. I uploaded and ran it on my ESP32 and recorded the freemem. Here's one set of measurements:

Sensor files Sensor Libraries Main.cpp Sensors RAM size Flash size Freemem Comments
No No Builtin 49 488 1 537 773 212 872
No All except AHRS Builtin 49 488 1 537 773 212 972
No All Builtin 49 488 1,537,773 212 400
All All Builtin 49 488 1 537 773 212 956 main.cpp had #includes for orientation files
All All Orientation 50 296 1 567 421 204 392 fusion filter Mahony (smallest)
All All Orientation 52 416 1 574 665 202 496 fusion filter Adafruit/NXP (biggest)

Note that it's only once the physical sensor gets used that the RAM use increases. There was no change regardless of whether or not the libraries were pulled in (as confirmed by inspecting the .pio folder), and whether or not the associated .h and .cpp files for the various hardware sensors were present or not. (There was one exception when I tested without the libraries - the DallasTemperature library was always present, as it is referenced by onewire_temperature.*, which is in turn referenced by wiring_helpers.* and I didn't do the surgery to sever that dependency).

d1_mini Board

To see whether it made a difference which board was being used, I repeated the above but with the platform.ini board selection set to d1_mini. I don't have a d1_mini, so am unable to report on the freemem values.

Sensor files Sensor Libraries Main.cpp Sensors RAM size Flash size Freemem Comments
No No Builtin 46 024 891 884 -
All All Builtin 46 024 891 884 -
All All Builtin 46 024 891 884 - main had #includes
All All Orientation 50740 924 176 - Adafruit/NXP filter
All All Orientation 47 672 914 460 - Mahony filter
All All Builtin 46 024 891 884 - back to no sensors

Next Steps

It would be appreciated if someone else could duplicate and confirm these results. Unless we can find a reason they are misleading, then it seems it really doesn't matter how many libraries and sensors are added to the SensESP project because under the current structure the only time a sensor's presence in the source code affects the memory use is if that sensor is actually invoked in main.cpp.

When testing with/without sensors, I used the orientation sensor as that's the one I'm most familiar with, and it's the one that's had the most fingers pointed at it. I reckon that if something amiss were to show up, it would happen with that one.

It's conceivable that some compiler setting(s) could influence these observations. I haven't knowingly changed any settings from the PlatformIO defaults and whatever they get set to when choosing an Arduino framework.

The likely explanations for increased RAM use that I can think of are:

I'd like to hear what others think. In the meantime, I'm less inclined to change the orientation sensor files as it seems like it wouldn't help with this current issue.

joelkoz commented 3 years ago

Good work! Additional output/debugging text could certainly contribute some. I remember early on us being very careful of putting large hunks of text in PROGMEM so it ends up in the flash count vs. the ram count (see https://arduino-esp8266.readthedocs.io/en/latest/PROGMEM.html). Personally, when it came to debug output, I was never very careful about it, and that pattern may have propagated. One solution would be to make our own debug macros like debugI() and debugE() that make sure the formatting text is wrapped in the PSTR() macro so it ends up in PROGMEM vs. regular RAM.

I am just now getting back into SensESP after being off for a while, so I can try to see if I can get the freemem numbers for a Wemos.

BjarneBitscrambler commented 3 years ago

I'm eager to hear how the Wemos tests out, Joel. I scrolled through this issue's history looking for nuggets that might help, and I read again the comment by @mairas (06 Nov) that showed a change in the freemem in the timeframe around mid-October. So I pulled down two commits bracketing that time, #75f723d and #5e4e30d and tested them the same way as above (i.e. with only built-in sensors, with and without the sensor source files and libraries). Interestingly, I didn't see a significant change in the freemem.

Possible differences between my test and Matti's include: I used the same main.cpp and platformio.ini each time (just commenting out the appropriate parts); and my test was on an ESP32 whereas Matti used an ESP8266. Don't know how those might affect the results - I'm out of time for today to look further into it.

The results were:

Sensor files Sensor Libraries Main.cpp Sensors RAM size Flash size Freemem Comments
Yes Yes Builtin 49 328 1 536 477 213 576 commit #75f723d
No No Builtin 49 328 1 536 477 213 596 commit #75f723d
Yes Yes Builtin 49 328 1 536 477 213 436 commit #5e4e30d
No No Builtin 49 328 1 536 477 213 576 commit #5e4e30d
mairas commented 3 years ago

My freemem testing didn't just show a change around mid-October, it was the specific commit when the magnetometer code was included. The ESP32 toolchain uses a newer gcc version and that might cause a change in behavior.

I'm summarizing as clearly I can: those large global variables must go. Alternatively, all of the magnetometer code can be split to a separately hosted library, after which the maintainer could do as they please.

BjarneBitscrambler commented 3 years ago

See PR #247. It compiles and runs fine on my ESP32 (and as noted above, makes zero difference in the freemen). If someone can test it on an ESP8266 and confirm that it saves RAM that would be great. Interesting about the gcc version - I hadn't realized they would be different between the ESP8266 and ESP32.

@mairas: You mention global variables (plural) several times. As far as I have seen in poking around the orientation code, there is only the one (i.e. the filter). Are you aware of others I should search for? I scrolled through PlatformIO's debug window listing of variables and didn't see any related to orientation, but again, that was on my ESP32.

joelkoz commented 3 years ago

@BjarneBitscrambler I happen to be looking at this right now just to familiarize myself with the code before doing some compiler/linker testing. SensorNXP_FXOS8700_FXAS21002* sensor_fxos_fxas, which is in orientation_9dof_input.cpp is another global variable. While it is true that it is a pointer vs. a static object, its mere existence would cause the linker to bring in all the Adafruit sensor code related to that structure.

I noticed that sensor_fxos_fxas was allocated dynamically, but filter was allocated statically. Is it possible that filter could be allocated dynamically also? That would prevent its memory usage for sensor apps not utilizing the orientation sensor.

Again, the other approach is to wrap both global variables (in both the .h and the .cpp file) with a define statement like #ifdef SENSOR_ORIENTATION9DOF, which would cause the compiler to be blind to their existence unless that symbol was defined in your platformio.ini file.

For my own curiosity, I am doing to do some more tests to see what the "dead code elimination" features of gcc are. This sensor's global variable is but one example of possible future problems if all sensor code is included in the main build.

joelkoz commented 3 years ago

@BjarneBitscrambler - actually, I see now that the dynamically allocated sensor_fxos_fxas is a reference to a different SensESP sensor code object.

The question about dynamic allocation still applies. If filter is allocated dynamically in the constructor of SensorNXP_FXOS8700_FXAS21002, that may go a long way toward helping the memory issue in the short term.

mairas commented 3 years ago

@BjarneBitscrambler I'm aware of only the filter object but I haven't read through the code in detail.

BjarneBitscrambler commented 3 years ago

Thanks Joel and Matti. The pointer that was global can now also be retired - see PR #249.

Any observations regarding the freemem available on the ESP8266, now that PR #247 was merged? I'm ordering a d1_mini for my own testing - seems like it will be useful.

ba58smith commented 3 years ago

@BjarneBitscrambler - make sure you get a "D1 mini", and not the "D1 mini Lite". The latter has only 1 meg of RAM, vs. 4 meg for the "D1 mini".

BjarneBitscrambler commented 3 years ago

After a hiatus I have a d1_mini (yay!), and here are some more observations to add to this memory-shortage thread:

BjarneBitscrambler commented 3 years ago

Another memory-saving observation:

zschramm commented 6 months ago

Hi, I've been trying to upload sensesp for a few days now but keep getting flash memory errors. At first I was trying with a NodeMCU and then second a YD-ESP32. I discovered that perhaps I did not have a board with enough memory as people say the 4 MB size is better for this platform. So I purchased a Wemos Mini D1 which is supposed to have 4 MB. I updated the platformio settings and even created a new project with the new board but I always get the same error and the same program size. I even commented out some of the libraries and other lines and the size is always 113% of the available memory.

image

I was trying to make a generic program to test connection with signalk based on some examples I found. As I said if comment out the max6675 library the build size is the same, assuming since I am not invoking it in the code. The end project was going to be a reader of some thermocouples for EGT and conversion to signalk and maybe directly into NMEA2k.

platformio.ini [env:wemos_d1_mini32] platform = espressif32 board = wemos_d1_mini32 framework = arduino lib_deps = adafruit/MAX6675 library@^1.1.2 signalk/SensESP@^2.7.2 monitor_speed = 9600

main.cpp `

include

include

include "sensesp/signalk/signalk_output.h"

include "sensesp_app.h"

include "sensesp_app_builder.h"

using namespace sensesp;

reactesp::ReactESP app;

float blackwaterlevel = 0.54; float blackwaterlevel_callback() { return (blackwaterlevel);}

void setup() {

ifndef SERIAL_DEBUG_DISABLED

SetupSerialDebug(9600);

endif

// Construct the global SensESPApp() object SensESPAppBuilder builder; sensesp_app = (&builder) // Set a custom hostname for the app. ->set_hostname("egt-temp") // Optionally, hard-code the WiFi and Signal K server // settings. This is normally not needed. //->set_wifi("My WiFi SSID", "my_wifi_password") //->set_sk_server("192.168.10.3", 80) ->get_app();

auto* blackwater_level = new RepeatSensor(1000, blackwaterlevel_callback);

blackwater_level->connect_to(new SKOutputFloat("tanks.blackwater.1.currentLevel"));

// Start networking, SK server connections and other SensESP internals sensesp_app->start(); }

void loop() { app.tick(); } '

Appreciate any help you can provide. Zac

ba58smith commented 6 months ago

@zschramm, the Wemos D1 Mini is an ESP8266, not an ESP32. The 8266 used to work with SensESP, but support for it was dropped with the change to version 2, I think - at least a few years ago.

zschramm commented 6 months ago

@zschramm, the Wemos D1 Mini is an ESP8266, not an ESP32. The 8266 used to work with SensESP, but support for it was dropped with the change to version 2, I think - at least a few years ago.

Thank you Brian for your quick response. This is what I ordered when I thought my ESP32 memory was just too small.

image

and this is what I think I bought image

Sorry if I didn't specify that it was a 32 based board (I think).

mairas commented 6 months ago

Given that the platformio.ini shows the board as wemos_d1_mini32, I assume there's a 32-bit version out there. But in any case, the reason for your problems is that you've dropped out the following line that defines the flash memory partition layout:

board_build.partitions = min_spiffs.csv

While at it, fix the monitor speed, too:

monitor_speed = 115200

And usually it's safe to keep the default board type (unless you're using an ESP32-Cx or ESP32-Sx device):

board = esp32dev

The last one has no effect, the middle one would've been your next problem, and I can explain the memory problem a bit. :-)

SensESP is indeed a big piece of software. It links in a lot of external libraries but also includes gzipped files served by the http server. The 4 MB flash memory is usually split into three major pieces: two identical partitions for the application code and one for the (spiffs) file system. The two application code partitions are used for over-the-air updates: uploaded code is written to the inactive partition and if the upload was successful, the active partition is switched over. The min_spiffs layout enlarges the app partition size at the cost of the file system data. There are other possible layouts as well. See here:

https://github.com/espressif/arduino-esp32/blob/master/tools/partitions/min_spiffs.csv

mairas commented 6 months ago

Oh, and I believe the boards you already had also have 4 MB of memory. They pretty much always do, except for newer ones 16 MB is becoming more common.

zschramm commented 6 months ago

Wow Mairas, you are the man! Everything uploaded now, thanks so much.

I created a new project in platformio (just learning how to use it) and it didn't include the mini_spiffs line by default. I guess I need to look up the board parameters.

mairas commented 6 months ago

I think the flash memory size is printed on the metal shield.

Anyways, I'll close the issue given the the problem was fixed.