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

upgrade to esp-wifi 0.9.1 #51

Merged
merged 3 commits into from
Oct 4, 2024

Conversation

yanshay
Copy link
Contributor

@yanshay yanshay commented Oct 3, 2024

removed duplicate functions
increased heap memory
updated examples cargo.toml

removed duplicate functions
increased heap memory
updated examples cargo.toml
on esp32 the increased size of the heap does not compile.
@yanshay
Copy link
Contributor Author

yanshay commented Oct 3, 2024

Can this be merged or should I change anything?

It's important since with esp-wifi 0.8.0 I encountered issues with getting IP, which I don't see with 0.9.1, and also there's a fix coming in the next versions to resolve some other issues getting IP, so need to get closer to vNext.

@bjoernQ
Copy link
Collaborator

bjoernQ commented Oct 4, 2024

Thanks!

StrBuf::from in compat/mod.rs seems to be unused now and can get removed.

The examples should be changed to use esp-alloc as a global allocator like here: https://github.com/esp-rs/esp-hal/blob/c7a236884589f33fe59b302154dfaa18c9af110c/examples/src/bin/wifi_dhcp.rs#L52

@yanshay
Copy link
Contributor Author

yanshay commented Oct 4, 2024

StrBuf::from in compat/mod.rs seems to be unused now and can get removed.

Done.

The examples should be changed to use esp-alloc as a global allocator like here: https://github.com/esp-rs/esp-hal/blob/c7a236884589f33fe59b302154dfaa18c9af110c/examples/src/bin/wifi_dhcp.rs#L52

I'm not familiar with this. Should it just be added?
I don't see the examples explicitly using alloc, and from my testing it seems like esp-mbedtls use the esp-wifi memory management, which I think in version 0.9.1 is still tuned in cfg.toml .

I noticed things changed post 0.9.1 but esp-mbedtls is tied to specific esp-hal and esp-wifi versions, so things coming in later versions won't get updated automatically (unfortunately).

Or maybe I'm missing something?

@bjoernQ
Copy link
Collaborator

bjoernQ commented Oct 4, 2024

I don't see the examples explicitly using alloc,

ah sorry - you are right that change is not yet released

@bjoernQ
Copy link
Collaborator

bjoernQ commented Oct 4, 2024

I guess the fact the examples don't work currently because of the heap-size is a bit unfortunate - after the next release we have more RAM to use so maybe it's fine for now

@AnthonyGrondin what do you think?

@yanshay
Copy link
Contributor Author

yanshay commented Oct 4, 2024

The examples do work on esp32s3 if I increase the heap size, but in the build it compiles the esp32 and that compile fail with a larger heap size.
It is no different I think than before my PR since inside the cfg.toml it says to use  minimum but the file is below that minimum.
I can also say that with esp-wifi 0.9.1 I had to increase esp-wifi memory to get esp-mbedtls to work. Luckily for me I can use psram for my app heap so I have memory to spare. But I think a lower memory TLS is becoming really critical. I know its in work, just saying it could make esp-hal unusable for a lot of applications as TLS is not something one can do without if it is required.

@bjoernQ
Copy link
Collaborator

bjoernQ commented Oct 4, 2024

I can also say that with esp-wifi 0.9.1 I had to increase esp-wifi memory to get esp-mbedtls to work.

Yes since 0.9.0 a few things which used to be allocated statically are now allocated on the heap - so that's fine

@AnthonyGrondin
Copy link
Contributor

I guess the fact the examples don't work currently because of the heap-size is a bit unfortunate - after the next release we have more RAM to use so maybe it's fine for now

@AnthonyGrondin what do you think?

I'm fine with any outcome. Some examples won't compile with 0.9.0 due to it being slightly larger, and with a static heap, we're at the maximum reach of what we can do with the default partition table.

I have somewhere a branch where I updated to the latest HEAD for esp-hal + esp-wifi to use the alloc system and the new init system, and it frees up a lot of memory, by sharing the same heap and not esp-wifi having its own static heap.

Once esp-rs/esp-hal#2137 is in a new release, I'd like to tackle the issues of memory usage, now that it would be easier to do so.

@bjoernQ bjoernQ merged commit 63eabf6 into esp-rs:main Oct 4, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants