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

Add option to use the main app's heap #1609

Closed
bugadani opened this issue Oct 25, 2023 · 14 comments · Fixed by #2099
Closed

Add option to use the main app's heap #1609

bugadani opened this issue Oct 25, 2023 · 14 comments · Fixed by #2099
Assignees
Labels
package:esp-wifi Issues related to the esp-wifi package
Milestone

Comments

@bugadani
Copy link
Contributor

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 :)

@bjoernQ
Copy link
Contributor

bjoernQ commented Oct 26, 2023

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

@bugadani
Copy link
Contributor Author

bugadani commented Oct 26, 2023

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
Copy link
Contributor Author

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?

@bjoernQ
Copy link
Contributor

bjoernQ commented Oct 29, 2023

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

@Volkalex28
Copy link
Contributor

@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

@bjoernQ
Copy link
Contributor

bjoernQ commented Jan 16, 2024

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

@Volkalex28
Copy link
Contributor

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

@Volkalex28
Copy link
Contributor

Volkalex28 commented Jan 16, 2024

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: esp-rs/esp-wifi-sys#415

@Volkalex28
Copy link
Contributor

Static allocation is fixed by lto="thin"

@bugadani
Copy link
Contributor Author

Static allocation is fixed by lto="thin"

With what lto setting did you experience the panic?

@Volkalex28
Copy link
Contributor

Volkalex28 commented Jan 16, 2024

@bugadani false

@bugadani
Copy link
Contributor Author

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

@Volkalex28
Copy link
Contributor

Sorry. Corrected previous message

@Volkalex28
Copy link
Contributor

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

@bjoernQ bjoernQ transferred this issue from esp-rs/esp-wifi-sys May 27, 2024
@bjoernQ bjoernQ added the package:esp-wifi Issues related to the esp-wifi package label May 27, 2024
@tom-borcin tom-borcin added this to the 0.21.0 milestone Aug 27, 2024
@bjoernQ bjoernQ self-assigned this Sep 5, 2024
@bjoernQ bjoernQ linked a pull request Sep 6, 2024 that will close this issue
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:esp-wifi Issues related to the esp-wifi package
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants