esp-wifi icon indicating copy to clipboard operation
esp-wifi copied to clipboard

Add option to use the main app's heap

Open bugadani opened this issue 1 year ago • 14 comments

I don't propose for this to be the only way, as heap fragmentation might bite some use cases, but it might not be a bad tool - I sometimes have 100k or so RAM I can lend to the wifi stack :)

bugadani avatar Oct 25 '23 18:10 bugadani

Wouldn't just increasing the WiFi heap (and lowering free RAM) be the same or did I miss something?

bjoernQ avatar Oct 26 '23 08:10 bjoernQ

Right now I have two heaps. Their allocation (i.e. how much is assigned to wifi and how much to my app) can't really change based on what state my firmware is in. Using a single heap would probably make things just a bit more flexible - no memory reserved for wifi when not needed, etc.

I guess an alternative solution could be registering esp-wifi's heap as the global allocator, if needed.

bugadani avatar Oct 26 '23 08:10 bugadani

Speaking of heaps, there are a bunch of things the API would like to allocate, but we provide static objects instead. There are a few assumption in code (like queue size of 200) that aren't really elegant and could instead be allocated on heap. Are we opposed to doing this, or are the current static objects mostly just quick and dirty solutions?

bugadani avatar Oct 28 '23 21:10 bugadani

TL;DR the initial implementation was to just get things going - I'm aware these are naive implementations

The initial implementation originates from me trying to get WiFi and BLE working on another RV32 SoC (it worked) and was more like a POC - but it helped a lot to get this working and a lot of the not so nice code here is just copied from there.

I planned on refactoring many of those naive implementations but then got hit by a lot of problems caused by the Xtensa enabled compiler. I think that should be fine now since the compiler got much, much better recently but I didn't get to work on that yet

bjoernQ avatar Oct 29 '23 11:10 bjoernQ

@bjoernQ How about adding the alloc feature? Then a bunch of wifi will use the app's global alocator. The question is whether there will be any problems if this alocator is configured on psram

Volkalex28 avatar Jan 16 '24 09:01 Volkalex28

Probably allocating in PSRAM would get us into problems, yes. And the allocator-API doesn't allow us to ask for "special" kinds of memory

Some opt-in-feature would still be fine (with a nice warning comment). In any case we should stay "no-alloc" (i.e. not needing a global allocator) by default

bjoernQ avatar Jan 16 '24 09:01 bjoernQ

Of course, by default there will be “its own” heap. I'm now trying to add this functionality. I'll post the results

Volkalex28 avatar Jan 16 '24 09:01 Volkalex28

Well

PSRAM

  • BLE works, but after some time it panics during deallocation
  • The station connects to WiFi (I see it in the router clients) but the program says that it cannot connect)
  • AP doesn't start (I don't see it in the available ones), but the app says it's up

Static alocator (practically the same as in the crate, only on the application side) I'm getting panic from here

0x4206d545 - wpa_config_bss
     at /home/bjoern/esp/esp-idf/components/wpa_supplicant/esp_supplicant/src/esp_wpa_main.c:109

This is not clear to me at all

For the allocator I used esp-alloc

This is draft changes: https://github.com/esp-rs/esp-wifi/pull/415

Volkalex28 avatar Jan 16 '24 11:01 Volkalex28

Static allocation is fixed by lto="thin"

Volkalex28 avatar Jan 16 '24 17:01 Volkalex28

Static allocation is fixed by lto="thin"

With what lto setting did you experience the panic?

bugadani avatar Jan 16 '24 17:01 bugadani

@bugadani false

Volkalex28 avatar Jan 16 '24 17:01 Volkalex28

I mean specifically what was the old setting out of fat / true / false / off?

bugadani avatar Jan 16 '24 17:01 bugadani

Sorry. Corrected previous message

Volkalex28 avatar Jan 16 '24 17:01 Volkalex28

If use psram, Wi-Fi does not work. I think need to somehow separate the allocations that can be made in psram and those that should be static

Volkalex28 avatar Jan 16 '24 17:01 Volkalex28