esp: Wi-Fi support #861
esp: Wi-Fi support #861
Conversation
- Fix idle task stack overflow - Add alignment to data sections - Upgrade `radio/timer.zig` to use a separate task - Add max value to semaphore
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔍 Lint Results
Found 9 issues on changed lines in 9 files:
- examples/espressif/esp/src/tcp_server.zig: 1 issue
- examples/raspberrypi/rp2xxx/src/net/lwip/net.zig: 1 issue
- port/espressif/esp/src/hal/radio/osi.zig: 1 issue
- port/espressif/esp/src/hal/system.zig: 1 issue
- port/raspberrypi/rp2xxx/src/hal/usb.zig: 1 issue
- port/stmicro/stm32/src/hals/STM32F103/adc.zig: 1 issue
- port/stmicro/stm32/src/hals/STM32F103/i2c.zig: 1 issue
- port/stmicro/stm32/src/hals/STM32F103/rcc.zig: 1 issue
- port/stmicro/stm32/src/hals/common/i2c_v2.zig: 1 issue
github-actions
bot
dismissed
their stale review
Updating with new lint results
|
I managed to crash the formatter :) The problematic file is |
github-actions
bot
dismissed
their stale review
Updating with new lint results
github-actions
bot
dismissed
their stale review
Updating with new lint results
|
|
||
| .data : | ||
| { | ||
| . = ALIGN(16); |
There was a problem hiding this comment.
I get littering the linker with align directives, but do you actually know for sure that these need align 16, or are you just going this high to be safe?
port/espressif/esp/src/hal/rtos.zig
Outdated
| const systimer = @import("systimer.zig"); | ||
|
|
||
| // How it works? | ||
| // For simple task to task context switches, only necessary registers are |
There was a problem hiding this comment.
Only the registers that the compiler wants to save. This happens through the clobbers from the inline asm context_switch.
There was a problem hiding this comment.
Yeah I mean to say that you should be more specific in the comment. It's defined by the ABI?
There was a problem hiding this comment.
Is it better now?
| }, | ||
| }; | ||
|
|
||
| const SSID = "SSID"; |
There was a problem hiding this comment.
Maybe add a comment here and a compile error if it's not set
There was a problem hiding this comment.
I'll add a comment. If we add a compile error, wouldn't CI fail?
| // TODO: config | ||
| const coex_enabled: bool = false; | ||
|
|
||
| pub var gpa: std.mem.Allocator = undefined; |
There was a problem hiding this comment.
nit but GPA implies general purpose allocator. Do they normally call it GPA unless it's actually that (or the debug allocator now)?
There was a problem hiding this comment.
Afaik, it is a good practice to name allocators gpa or arena to imply what kind of allocator do you need to provide. If it is an arena, you expect it to leak memory.
| tim.deadline = .init_relative(get_time_since_boot(), duration); | ||
| tim.periodic = if (repeat) duration else null; | ||
| } else { | ||
| log.warn("timer not found based on ets_timer", .{}); |
There was a problem hiding this comment.
Can we actually survive these? Should we be panicking if it's game over here rather than failing later on?
| }, | ||
| }; | ||
|
|
||
| const SSID = "SSID"; |
port/espressif/esp/src/hal/rtos.zig
Outdated
| const systimer = @import("systimer.zig"); | ||
|
|
||
| // How it works? | ||
| // For simple task to task context switches, only necessary registers are |
There was a problem hiding this comment.
Yeah I mean to say that you should be more specific in the comment. It's defined by the ABI?
tact1m4n3
force-pushed
the
esp_wifi_reloaded
branch
from
f161d13 to
599e2c4
Compare
github-actions
bot
dismissed
their stale review
Updating with new lint results
No description provided.