-
Notifications
You must be signed in to change notification settings - Fork 212
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
Move binary logging to sys crate #2183
Conversation
I'm tempted to write a test case, but the test case would be empty as I'm only going to check that the two drivers can be linked together. Do we think this is worth while? |
|
||
#[no_mangle] | ||
extern "C" fn rtc_clk_xtal_freq_get() -> i32 { | ||
0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was definitely wrong, I hope it wasn't breaking something 😅
4fcc1d5
to
e12cc18
Compare
I guess a test that just gets compiled and linked is nice - flashing a huge test which doesn't do anything is probably wasteful |
// libphy.a can pull this in on some chips, we provide it here in the hal | ||
// so that either ieee or esp-wifi's get it for free without duplicating in both | ||
#[no_mangle] | ||
extern "C" fn rtc_clk_xtal_freq_get() -> i32 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! I wanted to do this (or something similar) for quite some time but always forgot about it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Weeee, finally, we have real frequencies :D
a4fa6f1
to
19c430b
Compare
fa0e505
to
edfe617
Compare
We need to activate the |
29cc06b
to
78f5061
Compare
78f5061
to
f037e73
Compare
There is one new warning while building
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
Thank you for your contribution!
We appreciate the time and effort you've put into this pull request.
To help us review it efficiently, please ensure you've gone through the following checklist:
Submission Checklist 📝
cargo xtask fmt-packages
command to ensure that all changed code is formatted correctly.CHANGELOG.md
in the proper section.Extra:
Pull Request Details 📖
Closes #1932
Depends on esp-rs/esp-wifi-sys#477
Moves the logging methods to the sys crate to avoid duplicate definitions. ieee driver now takes
RADIO_CLK
by value, so that only wifi or IEEE can be enabled at once. In the future we'll want to relax this to usePeripheral
.