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.

Sign up for GitHub

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

Exposes core_id in the configuration of an HttpServer #534

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Exposes core_id in the configuration of an HttpServer #534

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0b9d2f3
Exposes core_id in the configuration of an HttpServer
Angelo13C Dec 24, 2024
513b18a
Merge branch 'esp-rs:master' into patch-1
Angelo13C Jan 2, 2025
6a2cb30
Update the http server to use the Core type instead of an int
Angelo13C Jan 2, 2025
168b32b
Fix name of variable
Angelo13C Jan 2, 2025
1d65e8f
Update CHANGELOG.md
Angelo13C Jan 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Change default eth key (#502)
- ESP IDF Partitions API (#511)
- Expose src_addr and dst_addr in espnow recv cb (#525)
- Add `core` field to `http::server::Configuration` to control which CPU core runs the HTTP server task
Angelo13C marked this conversation as resolved.
Show resolved Hide resolved

### Added
- Compatibility with ESP-IDF v5.3.X
Expand Down
6 changes: 5 additions & 1 deletion src/http/server.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ use embedded_svc::http::headers::content_type;
use embedded_svc::http::*;
use embedded_svc::io::{ErrorType, Read, Write};

use esp_idf_hal::cpu::Core;
Angelo13C marked this conversation as resolved.
Show resolved Hide resolved

use crate::sys::*;

use uncased::{Uncased, UncasedStr};
Expand All @@ -83,6 +85,7 @@ pub struct Configuration {
pub http_port: u16,
pub ctrl_port: u16,
pub https_port: u16,
pub core: Option<Core>,
pub max_sessions: usize,
pub session_timeout: Duration,
pub stack_size: usize,
Expand All @@ -103,6 +106,7 @@ impl Default for Configuration {
http_port: 80,
ctrl_port: 32768,
https_port: 443,
core: None,
max_sessions: 16,
session_timeout: Duration::from_secs(20 * 60),
#[cfg(not(esp_idf_esp_https_server_enable))]
Expand Down Expand Up @@ -141,7 +145,7 @@ impl From<&Configuration> for Newtype<httpd_config_t> {
))]
task_caps: (MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT),
stack_size: conf.stack_size,
core_id: i32::MAX,
core_id: conf.core.map(|core| core.into()).unwrap_or(i32::MAX),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This I'm not sure about because I don't know much about all this, but this should be a core still, no? Shouldn't you do something like this?

conf.core.map(|core| core.into()).unwrap_or(Core::Core0)

I'm not sure but it feels like i32::MAX would always crash.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, core_id is an i32, and i32::MAX works perfectly fine.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I checked, this is because of this definition

#define tskNO_AFFINITY      ( ( BaseType_t ) 0x7FFFFFFF )

So probably add a comment that says i32::MAX is to say no core affinity?

server_port: conf.http_port,
ctrl_port: conf.ctrl_port,
max_open_sockets: conf.max_open_sockets as _,
Expand Down
Loading