Enable support for espidf (esp32 family of microcontrollers)#245
Enable support for espidf (esp32 family of microcontrollers)#245newpavlov merged 4 commits intorust-random:masterfrom
Conversation
|
Shouldn't we prefer Also, would it be possible to add a build-only test for an ESP-IDF target to our CI config? |
|
That was my initial implementation, but then I found the esp-rs-compat repo and figured it was a better solution. Using esp_fill_random requires including a dependency on esp-idf-sys, which includes the burden on updating that dependency as time goes by. I have a version standing by using esp_fill_random, if that is preferred. As for the build test, I think this requires a patched rust compiler from espressif, so I'm not sure how to write that in a portable manner. |
|
@thorhs if you resync onto master you should be able to fix the CI issues.
This is a good point (about avoiding dependencies), which is why we generally just manually include the extern "C" {
fn esp_random() -> u32;
}In general, we try to use the lowest level RNG API available on a given platform (provided that it is safe and stable). So depending directly on the
@newpavlov's point is a good one, although using However, it seems like
If support for |
Import esp_fill_random by using extern "C" and skip depending of esp-idf-sys. Opted for esp_fill_random instead of esp_random since esp_fill_random implements logic for filling the u8 array efficiently from a u32 source. Figured it was more efficient than what I would implement.
There was a problem hiding this comment.
using esp_random avoids being able to inline the esp_random implementation into esp_fill_random, so it might not really matter
The most common use-case for getrandom is seeding of user-space PRNGs, which often use u32 or u64 as seeds. Using esp_random would allow generated data to stay in registers without round-tripping through stack. But I agree that it's a minuscule difference, so I am fine with esp_fill_random.
|
Could this also be merged into 0.1 branch? Should I submit a new PR for that? |
|
If you really need it in practice, then we could do it. But otherwise I don't think it's worth the trouble, since it would be better for your dependencies to use up-to-date |
Shamelessly forked from https://github.com/esp-rs-compat/getrandom.
Has been tested on my local MCU.