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

Start of Kobj support: Semaphores and Threads #12

Merged
merged 39 commits into from
Oct 17, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
fa00bea
zephyr::kconfig: Document module
d3zd3z Oct 11, 2024
823b48a
zephyr: Add some missing doc comments
d3zd3z Oct 11, 2024
45192f3
zephyr: Enforce documentation
d3zd3z Oct 11, 2024
ed875c2
zephyr: Add Error and Result types
d3zd3z Sep 23, 2024
e1196a0
zephyr: Make symbols from portable-atomic available
d3zd3z Sep 17, 2024
594bbc4
zephyr: Introduce static kobject support
d3zd3z Sep 18, 2024
3e362a7
zephyr: include portable atomic
d3zd3z Oct 11, 2024
67e456b
zephyr: Add sys::Sempahore
d3zd3z Oct 11, 2024
d8fbb57
zephyr: `take` in static kernel object takes args
d3zd3z Oct 11, 2024
aed43fd
zephyr: Provide critical-section for Zephyr
d3zd3z Sep 17, 2024
6598cd1
zephyr: Add alignment helper
d3zd3z Sep 23, 2024
a9ea9a1
zephyr: Add allocator support
d3zd3z Jul 18, 2024
59907d7
zephyr-sys: Export stack alignment constants
d3zd3z Sep 23, 2024
6b3e831
zephyr: Add sys::thread support
d3zd3z Oct 11, 2024
e71bd6a
samples: philosophers: Dining philosophers example
d3zd3z Oct 11, 2024
096d34b
Upgrade crate versions to match Zephyr
d3zd3z Oct 8, 2024
32b8fbf
zephyr: Add `k_uptime_get`
d3zd3z Sep 23, 2024
907c044
zephyr: Implement Debug for Semaphore
d3zd3z Oct 11, 2024
3d2477d
zephyr: StaticSemaphire needs to implement Sync
d3zd3z Oct 11, 2024
3fd043a
zephyr: Semaphore methods should not be `mut`
d3zd3z Oct 11, 2024
c88307f
zephyr: hide docs of RealStaticThreadStack
d3zd3z Oct 11, 2024
1b92ea5
zephyr: Expose some fields publicly
d3zd3z Oct 11, 2024
1971feb
zephyr: StaticThread must be Sync
d3zd3z Oct 11, 2024
97b217f
samples: hello_world: Fix dependency version
d3zd3z Oct 11, 2024
2ccb628
zephyr: Fix some warnings by being conditional
d3zd3z Oct 11, 2024
e8eebb2
samples: philosophers: Fix yaml
d3zd3z Oct 11, 2024
a34507f
tests: time: Fix dependency
d3zd3z Oct 11, 2024
91d6f33
zephyr-sys: Allow broken doc links in bindgen
d3zd3z Oct 12, 2024
177a932
zephyr: object: Improve docs about safety
d3zd3z Oct 12, 2024
1cd9299
zephyr: printk: Fix doc error
d3zd3z Oct 12, 2024
266a7a7
zephyr: sys: sync: Fix some doc links
d3zd3z Oct 12, 2024
fdd97ae
zephyr: printk: Remove dependency on alloc
d3zd3z Oct 14, 2024
d01153a
zephyr: Create export type alias
d3zd3z Oct 15, 2024
7375e44
zephyr: Use constructor for static thread stack
d3zd3z Oct 15, 2024
8bbade3
zephyr: object: Add kboj_define support for arrays of stacks
d3zd3z Oct 15, 2024
98a15fe
samples: philosophers: Convert to array of stacks
d3zd3z Oct 15, 2024
861cb83
zephyr: docgen
d3zd3z Oct 15, 2024
057e169
zephyr: Fix some broken doc comments
d3zd3z Oct 15, 2024
d11c5d2
module: Make sure all tests are run
d3zd3z Oct 15, 2024
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
14 changes: 6 additions & 8 deletions zephyr-build/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,24 +54,22 @@ pub fn build_kconfig_mod() {
let gen_path = Path::new(&outdir).join("kconfig.rs");

let mut f = File::create(&gen_path).unwrap();
writeln!(&mut f, "pub mod kconfig {{").unwrap();

let file = File::open(&dotconfig).expect("Unable to open dotconfig");
for line in BufReader::new(file).lines() {
let line = line.expect("reading line from dotconfig");
if let Some(caps) = config_hex.captures(&line) {
writeln!(&mut f, " #[allow(dead_code)]").unwrap();
writeln!(&mut f, " pub const {}: usize = {};",
writeln!(&mut f, "#[allow(dead_code)]").unwrap();
writeln!(&mut f, "pub const {}: usize = {};",
&caps[1], &caps[2]).unwrap();
} else if let Some(caps) = config_int.captures(&line) {
writeln!(&mut f, " #[allow(dead_code)]").unwrap();
writeln!(&mut f, " pub const {}: isize = {};",
writeln!(&mut f, "#[allow(dead_code)]").unwrap();
writeln!(&mut f, "pub const {}: isize = {};",
&caps[1], &caps[2]).unwrap();
} else if let Some(caps) = config_str.captures(&line) {
writeln!(&mut f, " #[allow(dead_code)]").unwrap();
writeln!(&mut f, " pub const {}: &'static str = {};",
writeln!(&mut f, "#[allow(dead_code)]").unwrap();
writeln!(&mut f, "pub const {}: &'static str = {};",
&caps[1], &caps[2]).unwrap();
}
}
writeln!(&mut f, "}}").unwrap();
Comment on lines -57 to -76
Copy link
Collaborator

Choose a reason for hiding this comment

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

This reads like it snuck into an unrelated commit? It does not seem to have anything to do with documentation?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It didn't sneak, it is very intentional. I needed to move the pub mod kconfig { up into lib.rs so that I could add both doc comments and directives to not fail on missing docs on the entries. To do that, I had to remove that from the generated code, and I unindented it as well.

}
1 change: 1 addition & 0 deletions zephyr-sys/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ fn main() -> Result<()> {
.allowlist_function("k_.*")
.allowlist_function("gpio_.*")
.allowlist_function("sys_.*")
.allowlist_item("E.*")
// Deprecated
.blocklist_function("sys_clock_timeout_end_calc")
.parse_callbacks(Box::new(bindgen::CargoCallbacks::new()))
Expand Down
56 changes: 56 additions & 0 deletions zephyr/src/error.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// Copyright (c) 2024 Linaro LTD
// SPDX-License-Identifier: Apache-2.0

//! # Zephyr errors
//!
//! This module contains an `Error` and `Result` type for use in wrapped Zephyr calls. Many
//! operations in Zephyr return an int result where negative values correspond with errnos.
//! Convert those to a `Result` type where the `Error` condition maps to errnos.
//!
//! Initially, this will just simply wrap the numeric error code, but it might make sense to make
//! this an enum itself, however, it would probably be better to auto-generate this enum instead of
//! trying to maintain the list manually.

use core::ffi::c_int;
use core::fmt;

// This is a little messy because the constants end up as u32 from bindgen, although the values are
// negative.

/// A Zephyr error.
///
/// Represents an error result returned within Zephyr.
pub struct Error(pub u32);

impl core::error::Error for Error {}

impl fmt::Display for Error {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "zephyr error errno:{}", self.0)
}
}

impl fmt::Debug for Error {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "zephyr error errno:{}", self.0)
}
}

/// Wraps a value with a possible Zephyr error.
pub type Result<T> = core::result::Result<T, Error>;

/// Map a return result from Zephyr into an Result.
///
/// Negative return results being considered errors.
pub fn to_result(code: c_int) -> Result<c_int> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is the c_int in the return needed for compatibility here? As this function already checks for negative values, I would find a uint more intuitive from an API usability perspective.

I would have also expected a convert to non FFI types, given that we are using the Rust Result, too?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The idea is to just keep the type as much like the underlying type, and any conversions can happen by the wrapper that is using this.

if code < 0 {
Err(Error(-code as u32))
} else {
Ok(code)
}
}

/// Map a return result, with a void result.
pub fn to_result_void(code: c_int) -> Result<()> {
to_result(code).map(|_| ())
}
Comment on lines +42 to +56
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder: Do these have to be pub? They seem to be mostly internal helpers? I find it to be slightly awkward API to be exposed to the outside world. So should this maybe be pub(crate)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Definitely a consideration. I still think they are useful for an app user that wants to use a binding that we haven't wrapped yet.

Copy link

Choose a reason for hiding this comment

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

Reducing the API may make it easier for people to use. This extra API seems to be a convenience API, but, at least for me, it just adds extra API for people who will use the crate to learn.

One possibility here would be to add a ZephyrCode type that could implement those methods. I think using the type Error is misleading because we are using it for error and also for normal returning code.

19 changes: 18 additions & 1 deletion zephyr/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,29 @@

#![no_std]
#![allow(unexpected_cfgs)]
#![deny(missing_docs)]

pub mod error;
pub mod sys;
pub mod time;

pub use error::{Error, Result};

// Bring in the generated kconfig module
include!(concat!(env!("OUT_DIR"), "/kconfig.rs"));
pub mod kconfig {
//! Zephyr Kconfig values.
//!
//! This module contains an auto-generated set of constants corresponding to the values of
//! various Kconfig values during the build.
//!
//! **Note**: Unless you are viewing docs generated for a specific build, the values below are
//! unlikely to directly correspond to those in a given build.

// Don't enforce doc comments on the bindgen, as it isn't enforced within Zephyr.
#![allow(missing_docs)]

include!(concat!(env!("OUT_DIR"), "/kconfig.rs"));
}

// Ensure that Rust is enabled.
#[cfg(not(CONFIG_RUST))]
Expand Down
22 changes: 22 additions & 0 deletions zephyr/src/printk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,33 @@ use core::fmt::{
write,
};

/// Print to Zephyr's console, without a newline.
///
/// This macro uses the same syntax as std's [`format!`], but writes to the Zephyr console instead.
///
/// if `CONFIG_PRINTK_SYNC` is enabled, this locks during printing. However, to avoid allocation,
/// and due to private accessors in the Zephyr printk implementation, the lock is only over groups
/// of a small buffer size. This buffer must be kept fairly small, as it resides on the stack.
///
/// [`format!`]: alloc::format
#[macro_export]
macro_rules! printk {
($($arg:tt)*) => {{
$crate::printk::printk(format_args!($($arg)*));
}};
}

/// Print to Zephyr's console, with a newline.
///
/// This macro uses the same syntax as std's
/// [`format!`], but writes to the Zephyr console
/// instead. See `std::fmt` for more information.
///
/// If `CONFIG_PRINTK_SYNC` is enabled, this locks during printing. However, to avoid allocation,
/// and due to private accessors in the Zephyr printk implementation, the lock is only over groups
/// of a small buffer size. This buffer must be kept fairly small, as it resides on the stack.
///
/// [`format!`]: alloc::format
#[macro_export]
macro_rules! printkln {
($($arg:tt)*) => {{
Expand Down Expand Up @@ -93,6 +113,7 @@ impl Write for Context {
}
}

#[doc(hidden)]
pub fn printk(args: Arguments<'_>) {
let mut context = Context {
count: 0,
Expand All @@ -102,6 +123,7 @@ pub fn printk(args: Arguments<'_>) {
context.flush();
}

#[doc(hidden)]
pub fn printkln(args: Arguments<'_>) {
let mut context = Context {
count: 0,
Expand Down
10 changes: 10 additions & 0 deletions zephyr/src/sys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,15 @@ use zephyr_sys::k_timeout_t;
// These two constants are not able to be captured by bindgen. It is unlikely that these values
// would change in the Zephyr headers, but there will be an explicit test to make sure they are
// correct.

/// Represents a timeout with an infinite delay.
///
/// Low-level Zephyr constant. Calls using this value will wait as long as necessary to perform
/// the requested operation.
pub const K_FOREVER: k_timeout_t = k_timeout_t { ticks: -1 };

/// Represents a null timeout delay.
///
/// Low-level Zephyr Constant. Calls using this value will not wait if the operation cannot be
/// performed immediately.
pub const K_NO_WAIT: k_timeout_t = k_timeout_t { ticks: 0 };
6 changes: 5 additions & 1 deletion zephyr/src/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,11 @@ pub type Instant = fugit::Instant<Tick, 1, SYS_FREQUENCY>;
// The absolute time offset is only implemented when time is a 64-bit value. This also means that
// "Instant" isn't available when time is defined as a 32-bit value.

// Wrapper around the timeout type, so we can implement From/Info.
/// Wrapper around the timeout type, so we can implement From/Info.
///
/// This wrapper allows us to implement `From` and `Info` from the Fugit types into the Zephyr
/// types. This allows various methods to accept either value, as well as the `Forever` and
/// `NoWait` values defined here.
pub struct Timeout(pub k_timeout_t);

// `From` allows methods to take a time of various types and convert it into a Zephyr timeout.
Expand Down