From 2af99de7e8cf46311764d0472dd559112b472120 Mon Sep 17 00:00:00 2001 From: chrysn Date: Wed, 22 Feb 2023 18:16:32 +0100 Subject: [PATCH 01/50] build: Use `syn` to pick out constants --- Cargo.toml | 1 + build.rs | 38 ++++++++++++++++++++++++++++++++------ 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index df8de3fb..9cca74d9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -48,6 +48,7 @@ pin-utils = "0.1" [build-dependencies] shlex = "0.1.1" +syn = { version = "1.0.107", features = [ "parsing" ] } [features] default = [] diff --git a/build.rs b/build.rs index 4d2f3a07..fd09b133 100644 --- a/build.rs +++ b/build.rs @@ -41,12 +41,38 @@ fn main() { let bindgen_output = std::fs::read_to_string(bindgen_output_file) .expect("Failed to read BINDGEN_OUTPUT_FILE"); - // Whether or not the extra space is there depends on whether or not rustfmt is installed. - // FIXME: Find a better way to extract that information - if bindgen_output.contains("pub const CONFIG_AUTO_INIT_ENABLE_DEBUG: u32 = 1;") - || bindgen_output.contains("pub const CONFIG_AUTO_INIT_ENABLE_DEBUG : u32 = 1 ;") - { - println!("cargo:rustc-cfg=marker_config_auto_init_enable_debug"); + const BOOLEAN_FLAGS: &[&str] = &[ + // This decides whether or not some fields are populated ... and unlike with other + // structs, the zeroed default is not a good solution here. (It'd kind of work, but + // it'd produce incorrect debug output). + "CONFIG_AUTO_INIT_ENABLE_DEBUG", + ]; + + let parsed = syn::parse_file(&bindgen_output).expect("Failed to parse bindgen output"); + for item in &parsed.items { + if let syn::Item::Const(const_) = item { + // It's the easiest way to get something we can `contains`... + let ident = const_.ident.to_string(); + if BOOLEAN_FLAGS.contains(&ident.as_str()) { + if let syn::Expr::Lit(syn::ExprLit { + lit: syn::Lit::Int(litint), + .. + }) = &*const_.expr + { + let value: usize = litint + .base10_parse() + .expect("Identifier is integer literal but not parsable"); + if value != 0 { + println!("cargo:rustc-cfg=marker_{}", ident.to_lowercase()); + } + continue; + } + panic!( + "Found {} but it's not the literal const it was expected to be", + ident + ); + } + } } } else { println!("cargo:warning=Old riot-sys did not provide BINDGEN_OUTPUT_FILE, assuming it's an old RIOT version"); From 57dda8f42ccf7e0bbb6529f395cfda7084084611 Mon Sep 17 00:00:00 2001 From: chrysn Date: Fri, 24 Feb 2023 13:22:06 +0100 Subject: [PATCH 02/50] saul: Follow C renamings --- src/saul.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/saul.rs b/src/saul.rs index 1e44ff1b..f2cbf76c 100644 --- a/src/saul.rs +++ b/src/saul.rs @@ -419,13 +419,13 @@ impl Unit { riot_sys::UNIT_M => Some(Unit::M), riot_sys::UNIT_M2 => Some(Unit::M2), riot_sys::UNIT_M3 => Some(Unit::M3), - riot_sys::UNIT_G => Some(Unit::GForce), + riot_sys::UNIT_G_FORCE => Some(Unit::GForce), riot_sys::UNIT_DPS => Some(Unit::Dps), - riot_sys::UNIT_GR => Some(Unit::Gram), + riot_sys::UNIT_GRAM => Some(Unit::Gram), riot_sys::UNIT_A => Some(Unit::A), riot_sys::UNIT_V => Some(Unit::V), riot_sys::UNIT_W => Some(Unit::W), - riot_sys::UNIT_GS => Some(Unit::Gauss), + riot_sys::UNIT_GAUSS => Some(Unit::Gauss), riot_sys::UNIT_T => Some(Unit::T), riot_sys::UNIT_DBM => Some(Unit::Dbm), riot_sys::UNIT_COULOMB => Some(Unit::Coulomb), @@ -459,13 +459,13 @@ impl Unit { Some(Unit::M) => riot_sys::UNIT_M, Some(Unit::M2) => riot_sys::UNIT_M2, Some(Unit::M3) => riot_sys::UNIT_M3, - Some(Unit::GForce) => riot_sys::UNIT_G, + Some(Unit::GForce) => riot_sys::UNIT_G_FORCE, Some(Unit::Dps) => riot_sys::UNIT_DPS, - Some(Unit::Gram) => riot_sys::UNIT_GR, + Some(Unit::Gram) => riot_sys::UNIT_GRAM, Some(Unit::A) => riot_sys::UNIT_A, Some(Unit::V) => riot_sys::UNIT_V, Some(Unit::W) => riot_sys::UNIT_W, - Some(Unit::Gauss) => riot_sys::UNIT_GS, + Some(Unit::Gauss) => riot_sys::UNIT_GAUSS, Some(Unit::T) => riot_sys::UNIT_T, Some(Unit::Dbm) => riot_sys::UNIT_DBM, Some(Unit::Coulomb) => riot_sys::UNIT_COULOMB, From 220b7cd7b56bf2233d6c069ec0277282bdb4cc98 Mon Sep 17 00:00:00 2001 From: chrysn Date: Thu, 12 Oct 2023 23:54:13 +0200 Subject: [PATCH 03/50] gnrc: Fix address conversion build failure When building with_embedded_nal, a build regression was introduced in 0ecf87e765f7264e79226540b694eb50993c7882 through careless refactoring. This restores buildability. --- src/gnrc/ipv6.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gnrc/ipv6.rs b/src/gnrc/ipv6.rs index b6a42e36..d8d401ae 100644 --- a/src/gnrc/ipv6.rs +++ b/src/gnrc/ipv6.rs @@ -180,7 +180,7 @@ impl From for Address { #[cfg(feature = "with_embedded_nal")] impl From
for embedded_nal::Ipv6Addr { fn from(addr: Address) -> Self { - Self::from(self.raw()) + Self::from(*addr.raw()) } } From 547ad2cf1174389dfa7704ef82c58b322a210e95 Mon Sep 17 00:00:00 2001 From: chrysn Date: Fri, 13 Oct 2023 15:23:57 +0200 Subject: [PATCH 04/50] tokenparts: Rename attribute that is never read --- src/thread/tokenparts.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/thread/tokenparts.rs b/src/thread/tokenparts.rs index 6dabf227..6aa14652 100644 --- a/src/thread/tokenparts.rs +++ b/src/thread/tokenparts.rs @@ -228,7 +228,7 @@ impl InThread { pub fn promote(self, value: T) -> ValueInThread { ValueInThread { value, - in_thread: self, + _in_thread: self, } } } @@ -258,7 +258,7 @@ impl InIsr { #[cfg_attr(feature = "nightly_docs", fundamental)] pub struct ValueInThread { value: T, - in_thread: InThread, + _in_thread: InThread, } impl ValueInThread { From 78dc3346046a8433ae016756a63120e1eecb14bc Mon Sep 17 00:00:00 2001 From: chrysn Date: Fri, 13 Oct 2023 15:27:10 +0200 Subject: [PATCH 05/50] Fix unused variable warning in cfg dependent section --- src/thread/riot_c.rs | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/thread/riot_c.rs b/src/thread/riot_c.rs index 750936b0..b2abdf90 100644 --- a/src/thread/riot_c.rs +++ b/src/thread/riot_c.rs @@ -172,16 +172,19 @@ impl KernelPID { /// This is not backed by C functions (as most of the rest of this crate is), but rather a /// practical way to access struct members that may or may not be present in a build. pub fn stack_stats(&self) -> Result { - let thread = self.thread()?; #[cfg(riot_develhelp)] - return Ok(StackStats { - // This cast is relevant because different platforms (eg. native and arm) disagree on - // whether that's an i8 or u8 pointer. Could have made it c_char, but a) don't want to - // alter the signatures and b) it's easier to use on the Rust side with a clear type. - start: unsafe { (*thread).stack_start as _ }, - size: unsafe { (*thread).stack_size as _ }, - free: unsafe { riot_sys::thread_measure_stack_free((*thread).stack_start) } as usize, - }); + { + let thread = self.thread()?; + return Ok(StackStats { + // This cast is relevant because different platforms (eg. native and arm) disagree on + // whether that's an i8 or u8 pointer. Could have made it c_char, but a) don't want to + // alter the signatures and b) it's easier to use on the Rust side with a clear type. + start: unsafe { (*thread).stack_start as _ }, + size: unsafe { (*thread).stack_size as _ }, + free: unsafe { riot_sys::thread_measure_stack_free((*thread).stack_start) } + as usize, + }); + } #[cfg(not(riot_develhelp))] return Err(StackStatsError::InformationUnavailable); } From f3a34e8669e7957c1de00ce48135951e1f93079a Mon Sep 17 00:00:00 2001 From: chrysn Date: Fri, 13 Oct 2023 15:48:45 +0200 Subject: [PATCH 06/50] tests: Fix unused import warning --- tests/mutex/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/mutex/src/lib.rs b/tests/mutex/src/lib.rs index 0bda93f4..926b1374 100644 --- a/tests/mutex/src/lib.rs +++ b/tests/mutex/src/lib.rs @@ -3,7 +3,7 @@ use riot_wrappers::mutex::Mutex; use riot_wrappers::println; use riot_wrappers::riot_main; -use riot_wrappers::thread::{InThread, ValueInThread}; +use riot_wrappers::thread::InThread; riot_main!(main); From da5b6d6ef2fba9e4b2b01517d19e88b103774013 Mon Sep 17 00:00:00 2001 From: chrysn Date: Sun, 15 Oct 2023 18:03:18 +0200 Subject: [PATCH 07/50] gnrc: Fix unused use warning --- src/gnrc/netreg.rs | 3 +-- src/gnrc/pktbuf.rs | 3 ++- src/never.rs | 1 + src/saul.rs | 4 ---- 4 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/gnrc/netreg.rs b/src/gnrc/netreg.rs index f294afd8..a4d3a42f 100644 --- a/src/gnrc/netreg.rs +++ b/src/gnrc/netreg.rs @@ -1,7 +1,6 @@ +#[cfg(feature = "with_msg_v2")] use core::mem::MaybeUninit; -use riot_sys::{gnrc_netreg_entry_t, gnrc_netreg_register, gnrc_netreg_unregister, gnrc_nettype_t}; - use crate::error::NegativeErrorExt; // Transmuting the pointer into a Pktsnip does the right thing by treating it as a smart diff --git a/src/gnrc/pktbuf.rs b/src/gnrc/pktbuf.rs index 153001bd..e219ae11 100644 --- a/src/gnrc/pktbuf.rs +++ b/src/gnrc/pktbuf.rs @@ -11,7 +11,6 @@ use riot_sys::{ gnrc_pktbuf_realloc_data, gnrc_pktbuf_release_error, gnrc_pktsnip_t, - gnrc_udp_hdr_build, GNRC_NETERR_SUCCESS, }; @@ -138,6 +137,8 @@ impl Pktsnip { src: core::num::NonZeroU16, dst: core::num::NonZeroU16, ) -> Result, NotEnoughSpace> { + use riot_sys::gnrc_udp_hdr_build; + let snip = unsafe { gnrc_udp_hdr_build(self.ptr, src.into(), dst.into()) }; if snip == 0 as *mut _ { // All other errors are caught by the signature diff --git a/src/never.rs b/src/never.rs index 3c12f841..499a750b 100644 --- a/src/never.rs +++ b/src/never.rs @@ -3,6 +3,7 @@ /// /// From , adjusted for /// usability with pub interfaces by using a pub trait in a private module (sealing). +#[cfg(not(feature = "actual_never_type"))] use crate::helpers::*; #[cfg(not(feature = "actual_never_type"))] diff --git a/src/saul.rs b/src/saul.rs index 115fa924..841c87d4 100644 --- a/src/saul.rs +++ b/src/saul.rs @@ -17,12 +17,8 @@ //! //! [SAUL]: https://doc.riot-os.org/group__drivers__saul.html -use riot_sys as raw; -use riot_sys::libc; - use crate::error; use crate::helpers::PointerToCStr; -use crate::Never; use error::NegativeErrorExt; pub mod registration; From 93a4c989346f6f52b936d43e4acb8e45e501152b Mon Sep 17 00:00:00 2001 From: chrysn Date: Sun, 15 Oct 2023 18:19:38 +0200 Subject: [PATCH 08/50] gcoap: Fix unused mut --- src/gcoap.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gcoap.rs b/src/gcoap.rs index 3450bee1..296a671a 100644 --- a/src/gcoap.rs +++ b/src/gcoap.rs @@ -216,7 +216,7 @@ unsafe extern "C" fn link_encoder( let h: &H = unsafe { &*((*resource).context as *const _) }; let buf = buf as *mut u8; // cast away signedness of char - let mut buf = if buf.is_null() { + let buf = if buf.is_null() { None } else { Some(core::slice::from_raw_parts_mut(buf, buf_len as _)) From 80f44432c2acce8c2df0d22fbf43265ad9915b80 Mon Sep 17 00:00:00 2001 From: chrysn Date: Sun, 15 Oct 2023 18:34:32 +0200 Subject: [PATCH 09/50] embedded-nal: Rename unused arguments to _ versions --- src/socket_embedded_nal_tcp.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/socket_embedded_nal_tcp.rs b/src/socket_embedded_nal_tcp.rs index 33f8c81c..7668e5a1 100644 --- a/src/socket_embedded_nal_tcp.rs +++ b/src/socket_embedded_nal_tcp.rs @@ -107,8 +107,8 @@ impl<'a, const QUEUELEN: usize> TcpClientStack for Pin<&'a mut ListenStack Result<(), nb::Error> { panic!("A ListenStack can not connect out.") } @@ -116,7 +116,7 @@ impl<'a, const QUEUELEN: usize> TcpClientStack for Pin<&'a mut ListenStack true, + SocketImpl::Connection(_n) => true, _ => false, }) } @@ -206,13 +206,15 @@ impl<'a, const QUEUELEN: usize> TcpFullStack for Pin<&'a mut ListenStack Result<(), Self::Error> { + fn listen(&mut self, _sock: &mut Self::TcpSocket) -> Result<(), Self::Error> { // Done already in bind Ok(()) } fn accept( &mut self, - sock: &mut Self::TcpSocket, + // This is and can actually be ignored, because our stack object only serves a single + // listening socket. + _sock: &mut Self::TcpSocket, ) -> Result<(Self::TcpSocket, SocketAddr), nb::Error> { let mut sockptr = core::ptr::null_mut(); unsafe { From b65a0d64e76eba871bff6e6292881a3745683e9c Mon Sep 17 00:00:00 2001 From: chrysn Date: Sun, 15 Oct 2023 18:53:41 +0200 Subject: [PATCH 10/50] gcoap: Remove unused internal function --- src/gcoap.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/gcoap.rs b/src/gcoap.rs index 296a671a..d39fccab 100644 --- a/src/gcoap.rs +++ b/src/gcoap.rs @@ -344,7 +344,6 @@ pub trait WithLinkEncoder { } use riot_sys::{ - coap_get_total_hdr_len, coap_opt_add_opaque, coap_opt_add_uint, coap_opt_get_next, @@ -376,11 +375,6 @@ impl PacketBuffer { }) as u8 // odd return type in C } - /// Wrapper for coap_get_total_hdr_len - fn get_total_hdr_len(&self) -> usize { - (unsafe { coap_get_total_hdr_len(crate::inline_cast(self.pkt)) }) as usize - } - /// Wrapper for gcoap_resp_init /// /// As it is used and wrapped here, this makes GCOAP_RESP_OPTIONS_BUF bytes unusable, but From 280bf427d8039be402a103a614360aa2a0233e6e Mon Sep 17 00:00:00 2001 From: chrysn Date: Fri, 13 Oct 2023 15:16:29 +0200 Subject: [PATCH 11/50] tests/led: Simplify to fix warnings --- tests/led/src/lib.rs | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/tests/led/src/lib.rs b/tests/led/src/lib.rs index 73571e24..f0ca139c 100644 --- a/tests/led/src/lib.rs +++ b/tests/led/src/lib.rs @@ -1,28 +1,27 @@ #![no_std] -use riot_wrappers::led; +use riot_wrappers::led::LED; use riot_wrappers::riot_main; riot_main!(main); fn main() { - let mut led0 = riot_wrappers::led::LED::<0>::new(); - let mut led1 = riot_wrappers::led::LED::<1>::new(); - let mut led2 = riot_wrappers::led::LED::<2>::new(); - let mut led3 = riot_wrappers::led::LED::<3>::new(); - let mut led4 = riot_wrappers::led::LED::<4>::new(); - let mut led5 = riot_wrappers::led::LED::<5>::new(); - let mut led6 = riot_wrappers::led::LED::<6>::new(); - let mut led7 = riot_wrappers::led::LED::<7>::new(); + let mut led0 = LED::<0>::new(); + let mut led1 = LED::<1>::new(); + let mut led2 = LED::<2>::new(); + let mut led3 = LED::<3>::new(); + let mut led4 = LED::<4>::new(); + let mut led5 = LED::<5>::new(); + let mut led6 = LED::<6>::new(); + let mut led7 = LED::<7>::new(); let mut leds: [&mut dyn switch_hal::ToggleableOutputSwitch; 8] = [ &mut led0, &mut led1, &mut led2, &mut led3, &mut led4, &mut led5, &mut led6, &mut led7, ]; - use switch_hal::ToggleableOutputSwitch; loop { for i in 0..=255 { for j in 0..8 { if (i ^ (i - 1)) & (1 << j) != 0 { - leds[j].toggle(); + leds[j].toggle().unwrap(); } } } From a72a4ab39edf3530170df13f499d50b097455afe Mon Sep 17 00:00:00 2001 From: chrysn Date: Sun, 15 Oct 2023 19:30:56 +0200 Subject: [PATCH 12/50] Fix unused imports --- src/gnrc/netreg.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/gnrc/netreg.rs b/src/gnrc/netreg.rs index a4d3a42f..573bf1b2 100644 --- a/src/gnrc/netreg.rs +++ b/src/gnrc/netreg.rs @@ -1,6 +1,7 @@ #[cfg(feature = "with_msg_v2")] use core::mem::MaybeUninit; +#[cfg(feature = "with_msg_v2")] use crate::error::NegativeErrorExt; // Transmuting the pointer into a Pktsnip does the right thing by treating it as a smart From 28758b1ae220c17d7573821dfb4c1d9a51c8759b Mon Sep 17 00:00:00 2001 From: chrysn Date: Fri, 13 Oct 2023 15:44:29 +0200 Subject: [PATCH 13/50] Silence deprecation warning from pub-use --- src/thread.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/thread.rs b/src/thread.rs index 075e1d20..a82a28b3 100644 --- a/src/thread.rs +++ b/src/thread.rs @@ -23,6 +23,7 @@ pub use riot_c::*; mod tokenparts; #[cfg(doc)] pub use tokenparts::TokenParts; +#[allow(deprecated)] pub use tokenparts::{EndToken, InIsr, InThread, StartToken, TerminationToken, ValueInThread}; mod stack_stats; From 09520f0d85ba2bd1bd6ef10dcc0a15c8afe2298a Mon Sep 17 00:00:00 2001 From: chrysn Date: Sun, 15 Oct 2023 18:16:57 +0200 Subject: [PATCH 14/50] Fix deprecated direct access --- src/bluetil.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bluetil.rs b/src/bluetil.rs index 6ac771f0..36d4da15 100644 --- a/src/bluetil.rs +++ b/src/bluetil.rs @@ -31,7 +31,7 @@ pub enum Error { impl From for Error { fn from(e: crate::error::NumericError) -> Error { - match e.number as _ { + match e.number() as _ { riot_sys::BLUETIL_AD_NOTFOUND => Error::NotFound, riot_sys::BLUETIL_AD_NOMEM => Error::NoMem, _ => panic!("Invalid bluetil error"), From 1bfe708abf0691ac9c29055211ac58e845b957ba Mon Sep 17 00:00:00 2001 From: chrysn Date: Sun, 15 Oct 2023 18:55:05 +0200 Subject: [PATCH 15/50] saul: Enum variants that were legacy shifted towards constants need naming exceptions --- src/saul.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/saul.rs b/src/saul.rs index 841c87d4..9dbcafd6 100644 --- a/src/saul.rs +++ b/src/saul.rs @@ -400,8 +400,10 @@ impl Unit { // we can just switch over before they go. #[deprecated(note = "Use the GForce variant instead")] pub const G: Self = Unit::GForce; + #[allow(non_upper_case_globals)] #[deprecated(note = "Use the Gram variant instead")] pub const Gr: Self = Unit::Gram; + #[allow(non_upper_case_globals)] #[deprecated(note = "Use the Gauss variant instead")] pub const Gs: Self = Unit::Gauss; From d99c90b5736b35dd1fa0f2e8dd8235c483a974e0 Mon Sep 17 00:00:00 2001 From: chrysn Date: Sun, 15 Oct 2023 18:22:54 +0200 Subject: [PATCH 16/50] treewide/doc: Various fixes --- src/auto_init.rs | 4 ++-- src/gcoap.rs | 2 +- src/interrupt.rs | 2 +- src/main_module.rs | 13 ++++++------- src/saul/registration.rs | 7 ++++--- src/shell.rs | 8 ++++---- src/socket_embedded_nal_tcp.rs | 5 +++-- src/thread.rs | 10 +++++----- src/thread/stack_stats.rs | 2 +- src/thread/tokenparts.rs | 5 +++-- src/vfs.rs | 2 +- 11 files changed, 31 insertions(+), 29 deletions(-) diff --git a/src/auto_init.rs b/src/auto_init.rs index 31213848..40070013 100644 --- a/src/auto_init.rs +++ b/src/auto_init.rs @@ -1,6 +1,6 @@ //! Tools for declaring a function that is run during initialization //! -//! The [auto_init!] macro is this module's main product. +//! The [`auto_init!`](super::auto_init!) macro is this module's main product. /// Wrapper around [riot_sys::auto_init_module_t] /// @@ -13,7 +13,7 @@ impl AutoInitModule { /// Initializer for module auto-initialization /// /// Do not call this directly: Its result must be placed in a static in a special section in - /// memory, which is handled by the [`auto_init!`] macro. + /// memory, which is handled by the [`auto_init!`](super::auto_init!) macro. pub const fn new( init_function: extern "C" fn(), priority: u16, diff --git a/src/gcoap.rs b/src/gcoap.rs index d39fccab..66188374 100644 --- a/src/gcoap.rs +++ b/src/gcoap.rs @@ -259,7 +259,7 @@ impl<'a, H> SingleHandlerListener<'a, H> where H: 'a + Handler + WithLinkEncoder, { - /// Like [`new()`], but utilizing that the handler is also [WithLinkEncoder] and can thus influence + /// Like [`Self::new()`], but utilizing that the handler is also [WithLinkEncoder] and can thus influence /// what is reported when the default .well-known/core handler is queried. pub fn new_with_link_encoder( path: &'a core::ffi::CStr, diff --git a/src/interrupt.rs b/src/interrupt.rs index 642f684a..a458a232 100644 --- a/src/interrupt.rs +++ b/src/interrupt.rs @@ -5,7 +5,7 @@ //! * Utility functions can disable interrupts (creating critical sections), check whether //! interrupts are enabled or to determine whether code is executed in a thread or an ISR. //! -//! * Some functions (eg. [`ZTimer::set_ticks_during`](crate::ztimer::ZTimer::set_ticks_during)) +//! * Some functions (eg. [`ZTimer::set_ticks_during`](crate::ztimer::Clock::set_during)) //! take callbacks that will be called in an interrupt context. //! //! These are typechecked to be Send, as they are moved from the thread to the interrupt context. diff --git a/src/main_module.rs b/src/main_module.rs index 5e8c8e99..c0943517 100644 --- a/src/main_module.rs +++ b/src/main_module.rs @@ -1,6 +1,6 @@ //! Tools for providing a RIOT main function //! -//! The main contribution of this module is the [riot_main] macro. +//! The main contribution of this module is the [`riot_main!`](super::riot_main!) macro. //! //! The alternative to using that (other than doing it manually) is to have C code along with the //! Rust application that occupies the main function. @@ -10,6 +10,7 @@ //! C code. use crate::stdio::println; +use crate::thread::{EndToken, StartToken}; // General alternative to this module: Build the extern "C" main all the time and request that the // application implement a named function. I never got the main function to be carried to the @@ -55,9 +56,9 @@ impl T, T: Termination> UsableAsMain<[u8; 1]> for F { } } -impl crate::never::Never> Sealed<[u8; 2]> for F {} +impl crate::never::Never> Sealed<[u8; 2]> for F {} -impl crate::never::Never> UsableAsMain<[u8; 2]> for F { +impl crate::never::Never> UsableAsMain<[u8; 2]> for F { unsafe fn call_main(&self) -> i32 { // unsafe: By construction of the C main function this only happens at startup time // with a thread that hasn't done anything relevant before. @@ -67,11 +68,9 @@ impl crate::never::Never> UsableAsMain<[u8; } } -impl ((), crate::thread::EndToken)> Sealed<[u8; 3]> for F {} +impl ((), EndToken)> Sealed<[u8; 3]> for F {} -impl ((), crate::thread::EndToken)> UsableAsMain<[u8; 3]> - for F -{ +impl ((), EndToken)> UsableAsMain<[u8; 3]> for F { unsafe fn call_main(&self) -> i32 { // unsafe: By construction of the C main function this only happens at startup time // with a thread that hasn't done anything relevant before. diff --git a/src/saul/registration.rs b/src/saul/registration.rs index 29739768..74c264cd 100644 --- a/src/saul/registration.rs +++ b/src/saul/registration.rs @@ -33,8 +33,9 @@ pub trait Drivable: Sized { /// Set to true if `read` is implemented. /// /// Doing this on the type level (rather than having read and write return a more - /// differentiated error) allows the driver to point to the shared [riot_sys::saul_notsup] - /// handler rather than to monomorphize a custom erring handler for each device. + /// differentiated error) allows the driver to point to the shared [riot_sys::saul_read_notsup] + /// / [riot_sys::saul_write_notsup] handler rather than to monomorphize a custom erring handler + /// for each device. const HAS_READ: bool = false; /// Set to true if `write` is implemented. const HAS_WRITE: bool = false; @@ -50,7 +51,7 @@ pub trait Drivable: Sized { /// Set the state of an actuator, or reconfigure a sensor /// /// A &self is passed in on write because there could be concurrent access from multiple SAUL - /// users. One option of handling this is to implement Drivable for Mutex. + /// users. One option of handling this is to implement Drivable for `Mutex`. /// /// Note that due to the way SAUL is structured, the drivable can not know the number of /// entries which the user intended to set. The Drivable trait always builds the Rust Phydat diff --git a/src/shell.rs b/src/shell.rs index f5d86203..b1a768d7 100644 --- a/src/shell.rs +++ b/src/shell.rs @@ -2,8 +2,8 @@ //! //! This module can be used in two ways: //! -//! * Declare static commands using [static_command]; these only take a `fn` (not a closure) -//! because shell commands don't have an arg pointer. +//! * Declare static commands using [`static_command!`](crate::static_command!); these only take a +//! `fn` (not a closure) because shell commands don't have an arg pointer. //! //! This works even in RIOT modules that are included in a C application that starts a shell, and //! show up in shells created through Rust without explicit inclusion. @@ -20,8 +20,8 @@ //! argument. This does allow the Rust wrappers to "just so" use a closure as a command handler, //! but also needs a lot of code. //! -//! That complexity is not pulled in when only using [static_command] and running on an otherwise -//! empty command list. +//! That complexity is not pulled in when only using [`static_command!`](crate::static_command!) +//! and running on an otherwise empty command list. use crate::{mutex, stdio}; use core::ffi::CStr; diff --git a/src/socket_embedded_nal_tcp.rs b/src/socket_embedded_nal_tcp.rs index 7668e5a1..3959c94a 100644 --- a/src/socket_embedded_nal_tcp.rs +++ b/src/socket_embedded_nal_tcp.rs @@ -1,8 +1,9 @@ //! An implementation of the [embedded_nal] (Network Abstradtion Layer) TCP traits based on RIOT //! sockets //! -//! This is vastly distinct from [socket_embedded_nal] as it requires vastly different workarounds -//! (and because it was implemented when embedded-nal had already switched over to &mut stack). +//! This is vastly distinct from [the UDP version](crate::socket_embedded_nal) as it requires +//! vastly different workarounds (and because it was implemented when embedded-nal had already +//! switched over to &mut stack). //! //! ## Warning //! diff --git a/src/thread.rs b/src/thread.rs index a82a28b3..aa9e964a 100644 --- a/src/thread.rs +++ b/src/thread.rs @@ -2,11 +2,11 @@ //! //! ## Tokens //! -//! Some thread creation mechanisms (currently only [riot_main_with_tokens] and not those in here) -//! are "with tokens". With these, the zero-sized type [StartToken] is used to pass along the -//! information that the execution is currently happening in a thread, and more importantly that -//! some operations doable only once per thread (eg. setting up a message queue) have not yet -//! happed. +//! Some thread creation mechanisms (currently only +//! [`riot_main_with_tokens!`](crate::riot_main_with_tokens!) and not those in here) are "with +//! tokens". With these, the zero-sized type [StartToken] is used to pass along the information +//! that the execution is currently happening in a thread, and more importantly that some +//! operations doable only once per thread (eg. setting up a message queue) have not yet happed. //! //! When threads are created that way, they need to return an [EndToken] which ensures that //! no operations that preclude the termination of a thread have happened. diff --git a/src/thread/stack_stats.rs b/src/thread/stack_stats.rs index 65466e82..10fa655a 100644 --- a/src/thread/stack_stats.rs +++ b/src/thread/stack_stats.rs @@ -1,4 +1,4 @@ -/// Gathered information about a thread, returned by [KernelPID::stack_stats()]. +/// Gathered information about a thread, returned by [super::KernelPID::stack_stats()]. /// /// All accessors are unconditional, because the StackStats can't be obtained without develhelp in /// the first place. diff --git a/src/thread/tokenparts.rs b/src/thread/tokenparts.rs index 6aa14652..df0b7115 100644 --- a/src/thread/tokenparts.rs +++ b/src/thread/tokenparts.rs @@ -44,8 +44,9 @@ pub struct TokenParts { /// Claim that the current thread has not done anything yet that is covered by this type /// - /// Do not call yourself; this needs to be public because [riot_main_with_tokens] is a macro - /// and thus technically called from the main crate. + /// Do not call yourself; this needs to be public because + /// [`riot_main_with_tokens!`](crate::riot_main_with_tokens!) is a macro and thus technically + /// called from the main crate. pub unsafe fn new() -> Self { TokenParts { _not_send: PhantomData, diff --git a/src/vfs.rs b/src/vfs.rs index 39f40fda..0450969f 100644 --- a/src/vfs.rs +++ b/src/vfs.rs @@ -43,7 +43,7 @@ impl Stat { /// Parameter for seeking in a file /// -/// It is analogous to [std::io::SeekFrom]. +/// It is analogous to [std::io::SeekFrom](https://doc.rust-lang.org/std/io/enum.SeekFrom.html). #[derive(Debug)] pub enum SeekFrom { /// Seek to the given position from the start of the file From 5bb8849830e2a5528501cae24fc2ea557fac2cc2 Mon Sep 17 00:00:00 2001 From: chrysn Date: Fri, 13 Oct 2023 15:24:22 +0200 Subject: [PATCH 17/50] thread/doc: Add comment on ValueInThread not being Send any more --- src/thread/tokenparts.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/thread/tokenparts.rs b/src/thread/tokenparts.rs index df0b7115..ccd4afc8 100644 --- a/src/thread/tokenparts.rs +++ b/src/thread/tokenparts.rs @@ -254,6 +254,8 @@ impl InIsr { /// /// This does barely implement anything on its own, but the module implementing `T` might provide /// extra methods. +/// +/// This makes the wrapped value not `Send`. // Making the type fundamental results in ValueInThread<&Mutex> being shown at Mutex's page. #[derive(Copy, Clone)] #[cfg_attr(feature = "nightly_docs", fundamental)] From 15e0b0980c9c7436a30d58081a0976310a76f962 Mon Sep 17 00:00:00 2001 From: chrysn Date: Fri, 13 Oct 2023 15:37:47 +0200 Subject: [PATCH 18/50] thread: Remove intermediate module layer from constants, fixing warnings --- src/msg.rs | 2 +- src/thread/riot_c.rs | 110 ++++++++++++++++++++----------------------- 2 files changed, 52 insertions(+), 60 deletions(-) diff --git a/src/msg.rs b/src/msg.rs index c14afdb8..e99044c1 100644 --- a/src/msg.rs +++ b/src/msg.rs @@ -28,7 +28,7 @@ pub enum MsgSender { impl MsgSender { fn from_pid(pid: kernel_pid_t) -> Self { - if pid == crate::thread::pid_converted::KERNEL_PID_ISR { + if pid == crate::thread::KERNEL_PID_ISR { MsgSender::ISR } else { KernelPID::new(pid) diff --git a/src/thread/riot_c.rs b/src/thread/riot_c.rs index b2abdf90..4a93e5cb 100644 --- a/src/thread/riot_c.rs +++ b/src/thread/riot_c.rs @@ -14,41 +14,33 @@ pub use creation::{scope, spawn, CountedThread, CountingThreadScope}; #[derive(Debug, PartialEq, Copy, Clone)] pub struct KernelPID(pub(crate) raw::kernel_pid_t); -pub(crate) mod pid_converted { - //! Converting the raw constants into consistently typed ones - use riot_sys as raw; - - pub const KERNEL_PID_UNDEF: raw::kernel_pid_t = raw::KERNEL_PID_UNDEF as _; - pub const KERNEL_PID_FIRST: raw::kernel_pid_t = raw::KERNEL_PID_FIRST as _; - pub const KERNEL_PID_LAST: raw::kernel_pid_t = raw::KERNEL_PID_LAST as _; - pub const KERNEL_PID_ISR: raw::kernel_pid_t = raw::KERNEL_PID_ISR as _; -} - -mod status_converted { - //! Converting the raw constants into consistently typed ones for use in match branches. If - //! that becomes a pattern, it might make sense to introduce a macro that forces a bunch of - //! symbols (with different capitalizations) into a given type and makes an enum with a - //! from_int method out of it. - - use riot_sys as raw; - - // This is special because it is not part of the enum but a cast -1 - // unsafe: Side effect free C macros - pub const STATUS_NOT_FOUND: i32 = unsafe { raw::macro_STATUS_NOT_FOUND() as _ }; - - pub const STATUS_STOPPED: i32 = raw::thread_status_t_STATUS_STOPPED as i32; - pub const STATUS_SLEEPING: i32 = raw::thread_status_t_STATUS_SLEEPING as i32; - pub const STATUS_MUTEX_BLOCKED: i32 = raw::thread_status_t_STATUS_MUTEX_BLOCKED as i32; - pub const STATUS_RECEIVE_BLOCKED: i32 = raw::thread_status_t_STATUS_RECEIVE_BLOCKED as i32; - pub const STATUS_SEND_BLOCKED: i32 = raw::thread_status_t_STATUS_SEND_BLOCKED as i32; - pub const STATUS_REPLY_BLOCKED: i32 = raw::thread_status_t_STATUS_REPLY_BLOCKED as i32; - pub const STATUS_FLAG_BLOCKED_ANY: i32 = raw::thread_status_t_STATUS_FLAG_BLOCKED_ANY as i32; - pub const STATUS_FLAG_BLOCKED_ALL: i32 = raw::thread_status_t_STATUS_FLAG_BLOCKED_ALL as i32; - pub const STATUS_MBOX_BLOCKED: i32 = raw::thread_status_t_STATUS_MBOX_BLOCKED as i32; - pub const STATUS_RUNNING: i32 = raw::thread_status_t_STATUS_RUNNING as i32; - pub const STATUS_PENDING: i32 = raw::thread_status_t_STATUS_PENDING as i32; -} - +// Converting the raw constants into consistently typed ones + +// pub(crate) const KERNEL_PID_UNDEF: riot_sys::kernel_pid_t = riot_sys::KERNEL_PID_UNDEF as _; +const KERNEL_PID_FIRST: riot_sys::kernel_pid_t = riot_sys::KERNEL_PID_FIRST as _; +const KERNEL_PID_LAST: riot_sys::kernel_pid_t = riot_sys::KERNEL_PID_LAST as _; +pub(crate) const KERNEL_PID_ISR: riot_sys::kernel_pid_t = riot_sys::KERNEL_PID_ISR as _; + +// Converting the raw constants into consistently typed ones for use in match branches. If +// that becomes a pattern, it might make sense to introduce a macro that forces a bunch of +// symbols (with different capitalizations) into a given type and makes an enum with a +// from_int method out of it. + +// This is special because it is not part of the enum but a cast -1 +// unsafe: Side effect free C macros +const STATUS_NOT_FOUND: i32 = unsafe { riot_sys::macro_STATUS_NOT_FOUND() as _ }; + +const STATUS_STOPPED: i32 = riot_sys::thread_status_t_STATUS_STOPPED as i32; +const STATUS_SLEEPING: i32 = riot_sys::thread_status_t_STATUS_SLEEPING as i32; +const STATUS_MUTEX_BLOCKED: i32 = riot_sys::thread_status_t_STATUS_MUTEX_BLOCKED as i32; +const STATUS_RECEIVE_BLOCKED: i32 = riot_sys::thread_status_t_STATUS_RECEIVE_BLOCKED as i32; +const STATUS_SEND_BLOCKED: i32 = riot_sys::thread_status_t_STATUS_SEND_BLOCKED as i32; +const STATUS_REPLY_BLOCKED: i32 = riot_sys::thread_status_t_STATUS_REPLY_BLOCKED as i32; +const STATUS_FLAG_BLOCKED_ANY: i32 = riot_sys::thread_status_t_STATUS_FLAG_BLOCKED_ANY as i32; +const STATUS_FLAG_BLOCKED_ALL: i32 = riot_sys::thread_status_t_STATUS_FLAG_BLOCKED_ALL as i32; +const STATUS_MBOX_BLOCKED: i32 = riot_sys::thread_status_t_STATUS_MBOX_BLOCKED as i32; +const STATUS_RUNNING: i32 = riot_sys::thread_status_t_STATUS_RUNNING as i32; +const STATUS_PENDING: i32 = riot_sys::thread_status_t_STATUS_PENDING as i32; #[derive(Debug)] #[non_exhaustive] @@ -56,17 +48,17 @@ pub enum Status { // I would not rely on any properties of the assigned values, but it might make the conversion // points easier on the generated code if it can be reasoned down to a simple check of whether // it's in range. - Stopped = status_converted::STATUS_STOPPED as isize, - Sleeping = status_converted::STATUS_SLEEPING as isize, - MutexBlocked = status_converted::STATUS_MUTEX_BLOCKED as isize, - ReceiveBlocked = status_converted::STATUS_RECEIVE_BLOCKED as isize, - SendBlocked = status_converted::STATUS_SEND_BLOCKED as isize, - ReplyBlocked = status_converted::STATUS_REPLY_BLOCKED as isize, - FlagBlockedAny = status_converted::STATUS_FLAG_BLOCKED_ANY as isize, - FlagBlockedAll = status_converted::STATUS_FLAG_BLOCKED_ALL as isize, - MboxBlocked = status_converted::STATUS_MBOX_BLOCKED as isize, - Running = status_converted::STATUS_RUNNING as isize, - Pending = status_converted::STATUS_PENDING as isize, + Stopped = STATUS_STOPPED as isize, + Sleeping = STATUS_SLEEPING as isize, + MutexBlocked = STATUS_MUTEX_BLOCKED as isize, + ReceiveBlocked = STATUS_RECEIVE_BLOCKED as isize, + SendBlocked = STATUS_SEND_BLOCKED as isize, + ReplyBlocked = STATUS_REPLY_BLOCKED as isize, + FlagBlockedAny = STATUS_FLAG_BLOCKED_ANY as isize, + FlagBlockedAll = STATUS_FLAG_BLOCKED_ALL as isize, + MboxBlocked = STATUS_MBOX_BLOCKED as isize, + Running = STATUS_RUNNING as isize, + Pending = STATUS_PENDING as isize, /// A status value not known to riot-wrappers. Don't match for this explicitly: Other values /// may, at any minor riot-wrappers update, become actual process states again. @@ -78,17 +70,17 @@ pub enum Status { impl Status { fn from_int(status: i32) -> Self { match status { - status_converted::STATUS_STOPPED => Status::Stopped, - status_converted::STATUS_SLEEPING => Status::Sleeping, - status_converted::STATUS_MUTEX_BLOCKED => Status::MutexBlocked, - status_converted::STATUS_RECEIVE_BLOCKED => Status::ReceiveBlocked, - status_converted::STATUS_SEND_BLOCKED => Status::SendBlocked, - status_converted::STATUS_REPLY_BLOCKED => Status::ReplyBlocked, - status_converted::STATUS_FLAG_BLOCKED_ANY => Status::FlagBlockedAny, - status_converted::STATUS_FLAG_BLOCKED_ALL => Status::FlagBlockedAll, - status_converted::STATUS_MBOX_BLOCKED => Status::MboxBlocked, - status_converted::STATUS_RUNNING => Status::Running, - status_converted::STATUS_PENDING => Status::Pending, + STATUS_STOPPED => Status::Stopped, + STATUS_SLEEPING => Status::Sleeping, + STATUS_MUTEX_BLOCKED => Status::MutexBlocked, + STATUS_RECEIVE_BLOCKED => Status::ReceiveBlocked, + STATUS_SEND_BLOCKED => Status::SendBlocked, + STATUS_REPLY_BLOCKED => Status::ReplyBlocked, + STATUS_FLAG_BLOCKED_ANY => Status::FlagBlockedAny, + STATUS_FLAG_BLOCKED_ALL => Status::FlagBlockedAll, + STATUS_MBOX_BLOCKED => Status::MboxBlocked, + STATUS_RUNNING => Status::Running, + STATUS_PENDING => Status::Pending, _ => Status::Other, } } @@ -109,7 +101,7 @@ impl KernelPID { // and then this function *should* be reevaluated). As pid_is_valid is static inline, the // compiler should be able to see through the calls down to there that the bounds checked // for there are the very bounds used in the construction here. - (pid_converted::KERNEL_PID_FIRST..=pid_converted::KERNEL_PID_LAST) + (KERNEL_PID_FIRST..=KERNEL_PID_LAST) .map(|i| KernelPID::new(i).expect("Should be valid by construction")) } @@ -128,7 +120,7 @@ impl KernelPID { pub fn status(&self) -> Result { // unsafe: Side effect free, always-callable C function let status = unsafe { raw::thread_getstatus(self.0) } as _; - if status == status_converted::STATUS_NOT_FOUND { + if status == STATUS_NOT_FOUND { Err(NoSuchThread) } else { Ok(Status::from_int(status)) From d6de3cbe6e8cb735abfaa8b6c5ee79437fa30b70 Mon Sep 17 00:00:00 2001 From: chrysn Date: Sun, 15 Oct 2023 18:05:00 +0200 Subject: [PATCH 19/50] gnrc_pktbuf: Improve debugging experience by showing the writability generic in debug output --- src/gnrc/pktbuf.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/gnrc/pktbuf.rs b/src/gnrc/pktbuf.rs index 858a0da8..c67a8f7c 100644 --- a/src/gnrc/pktbuf.rs +++ b/src/gnrc/pktbuf.rs @@ -291,7 +291,13 @@ impl<'a> Pktsnip { impl ::core::fmt::Debug for Pktsnip { fn fmt(&self, f: &mut ::core::fmt::Formatter) -> ::core::fmt::Result { + let mode = core::any::type_name::(); + let mode = mode + .rsplit("::") + .next() + .expect("Type name contains :: because it is part of a module"); f.debug_struct("Pktsnip") + .field("M", &mode) .field("length", &self.len()) .field("snips", &self.count()) .finish() From afe025ccb52fe162d9eca1a67fc34ca975888942 Mon Sep 17 00:00:00 2001 From: chrysn Date: Fri, 13 Oct 2023 15:39:03 +0200 Subject: [PATCH 20/50] panic: Refactor The `unreachable!` previously acted as a safeguard in case the translation of C's "no return" indicator would not work, ensuring the code path is not followed any further -- but the linter didn't like the (currently) unreachable code. Instead, we now implicitly assert that the panic function does not return by not explicitly having a return in that branch. --- src/panic.rs | 67 ++++++++++++++++++++++++++-------------------------- 1 file changed, 33 insertions(+), 34 deletions(-) diff --git a/src/panic.rs b/src/panic.rs index f5cf78d9..41e33547 100644 --- a/src/panic.rs +++ b/src/panic.rs @@ -24,46 +24,45 @@ fn panic(info: &::core::panic::PanicInfo) -> ! { cstr::cstr!("RUST PANIC").as_ptr() as _, ) }; - unreachable!() - } - - // I *guess* it's OK for a panic to simply make a thread into a zombie -- this does allow other - // threads (including spawned Rust threads) to continue, but my layman's understanding of - // panicking is that that's OK because whatever we were just mutating can simply never be used - // by someone else ever again. + } else { + // I *guess* it's OK for a panic to simply make a thread into a zombie -- this does allow other + // threads (including spawned Rust threads) to continue, but my layman's understanding of + // panicking is that that's OK because whatever we were just mutating can simply never be used + // by someone else ever again. - let me = thread::get_pid(); + let me = thread::get_pid(); - if cfg!(feature = "panic_handler_format") { - use crate::stdio::println; + if cfg!(feature = "panic_handler_format") { + use crate::stdio::println; - println!( - "Error in thread {:?} ({}):", - me, - me.get_name().unwrap_or("unnamed") - ); - println!("{}", info); - } else { - let mut stdio = crate::stdio::Stdio {}; - use core::fmt::Write; - let _ = stdio.write_str("Panic in thread "); - let _ = stdio.write_str(me.get_name().unwrap_or("unnamed")); - let _ = stdio.write_str("!\n"); - } + println!( + "Error in thread {:?} ({}):", + me, + me.get_name().unwrap_or("unnamed") + ); + println!("{}", info); + } else { + let mut stdio = crate::stdio::Stdio {}; + use core::fmt::Write; + let _ = stdio.write_str("Panic in thread "); + let _ = stdio.write_str(me.get_name().unwrap_or("unnamed")); + let _ = stdio.write_str("!\n"); + } - if cfg!(feature = "panic_handler_crash") { - unsafe { - riot_sys::core_panic( - riot_sys::core_panic_t_PANIC_GENERAL_ERROR, - cstr::cstr!("RUST PANIC").as_ptr() as _, - ) + if cfg!(feature = "panic_handler_crash") { + unsafe { + riot_sys::core_panic( + riot_sys::core_panic_t_PANIC_GENERAL_ERROR, + cstr::cstr!("RUST PANIC").as_ptr() as _, + ) + } } - } - // Not trying any unwinding -- this thread is just dead, won't be re-claimed, any mutexes it - // holds are just held indefinitely rather than throwing poison errors. - loop { - thread::sleep(); + // Not trying any unwinding -- this thread is just dead, won't be re-claimed, any mutexes it + // holds are just held indefinitely rather than throwing poison errors. + loop { + thread::sleep(); + } } } From 6430de23e81ee483efbb662710fc91dceaf9f1ed Mon Sep 17 00:00:00 2001 From: chrysn Date: Sat, 14 Oct 2023 18:48:09 +0200 Subject: [PATCH 21/50] pktbuf: Fix double free The `forget()` call was on the (Copy, hence a warning that is being fixed) raw pointer instead of the underlying refcounted structure, leading to a double free. --- src/gnrc/pktbuf.rs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/gnrc/pktbuf.rs b/src/gnrc/pktbuf.rs index e219ae11..858a0da8 100644 --- a/src/gnrc/pktbuf.rs +++ b/src/gnrc/pktbuf.rs @@ -254,13 +254,19 @@ impl<'a> Pktsnip { size: usize, nettype: gnrc_nettype_t, ) -> Result { - let next = next.map(|s| s.ptr).unwrap_or(0 as *mut _); - let snip = - unsafe { gnrc_pktbuf_add(next, data as *const _, size.try_into().unwrap(), nettype) }; + let next_ptr = next.as_ref().map(|s| s.ptr).unwrap_or(0 as *mut _); + forget(next); + let snip = unsafe { + gnrc_pktbuf_add( + next_ptr, + data as *const _, + size.try_into().unwrap(), + nettype, + ) + }; if snip == 0 as *mut _ { return Err(NotEnoughSpace); } - forget(next); Ok(unsafe { Pktsnip::::from_ptr(snip) }) } From bb73bf83d061d5ba4f30ae92df54b210dc455fb2 Mon Sep 17 00:00:00 2001 From: chrysn Date: Sun, 15 Oct 2023 18:18:44 +0200 Subject: [PATCH 22/50] saul: Don't abuse negative errors with positive values --- src/saul.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/saul.rs b/src/saul.rs index 9dbcafd6..951b72f6 100644 --- a/src/saul.rs +++ b/src/saul.rs @@ -91,10 +91,9 @@ impl RegistryEntry { unsafe { riot_sys::saul_reg_write(self.0, &value.values as *const _ as *mut _) } .negative_to_error()?; if length != value.length.into() { - // FIXME is this the best way to express the error? - Err(error::NumericError { - number: length as isize, - }) + // It's not pretty to synthesize an error here, but neither would be expressing the + // partial write in the context of lengthful phydat items. + Err(error::NumericError::from_constant(riot_sys::EINVAL as _)) } else { Ok(()) } From f230e66ac6be146bbe32df4772ba1610f4e0f6b6 Mon Sep 17 00:00:00 2001 From: chrysn Date: Sun, 15 Oct 2023 19:08:02 +0200 Subject: [PATCH 23/50] shell: Refactor to avoid lint complaining about dropping reference The lint is right in that dropping the reference does little, but it ensures that the reference is not held longer than the lock. Using an explicit scope achieves the same without any discussions about the drop. --- src/shell.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/shell.rs b/src/shell.rs index b1a768d7..06a25d34 100644 --- a/src/shell.rs +++ b/src/shell.rs @@ -108,11 +108,13 @@ pub unsafe trait CommandListInternals: Sized { let sleeve = lock .as_ref() .expect("Callback called while no shell set up as running"); - // unsafe: A suitable callback is always configured. We can make a &mut out of it for as - // long as we hold the lock. - let root = unsafe { &mut *(sleeve.0 as *mut Self) }; - let result = root.find_self_and_run(argc, argv, command_index); - drop(root); + let result; + { + // unsafe: A suitable callback is always configured. We can make a &mut out of it for as + // long as we hold the lock. + let root = unsafe { &mut *(sleeve.0 as *mut Self) }; + result = root.find_self_and_run(argc, argv, command_index); + } drop(lock); result } From 8f42354b9a033dd92ee825aa2429a27e34882ffa Mon Sep 17 00:00:00 2001 From: chrysn Date: Sun, 15 Oct 2023 17:05:32 +0200 Subject: [PATCH 24/50] tests: Add gnrc-pktbuf --- tests/gnrc-pktbuf/Cargo.toml | 19 +++++++++++ tests/gnrc-pktbuf/Makefile | 14 ++++++++ tests/gnrc-pktbuf/src/lib.rs | 57 +++++++++++++++++++++++++++++++ tests/gnrc-pktbuf/tests/01-run.py | 10 ++++++ 4 files changed, 100 insertions(+) create mode 100644 tests/gnrc-pktbuf/Cargo.toml create mode 100644 tests/gnrc-pktbuf/Makefile create mode 100644 tests/gnrc-pktbuf/src/lib.rs create mode 100755 tests/gnrc-pktbuf/tests/01-run.py diff --git a/tests/gnrc-pktbuf/Cargo.toml b/tests/gnrc-pktbuf/Cargo.toml new file mode 100644 index 00000000..b23236f4 --- /dev/null +++ b/tests/gnrc-pktbuf/Cargo.toml @@ -0,0 +1,19 @@ +[package] +name = "riot-wrappers-test-gnrc-pktbuf" +version = "0.1.0" +authors = ["Christian Amsüss "] +edition = "2021" +publish = false + +[lib] +crate-type = ["staticlib"] + +[profile.release] +panic = "abort" + +[dependencies] +riot-wrappers = { version = "*", features = [ "set_panic_handler", "panic_handler_format" ] } +riot-sys = "*" + +[patch.crates-io] +riot-sys = { git = "https://github.com/RIOT-OS/rust-riot-sys" } diff --git a/tests/gnrc-pktbuf/Makefile b/tests/gnrc-pktbuf/Makefile new file mode 100644 index 00000000..d31c9f22 --- /dev/null +++ b/tests/gnrc-pktbuf/Makefile @@ -0,0 +1,14 @@ +# name of your application +APPLICATION = riot-wrappers-test-gnrc-pktbuf +APPLICATION_RUST_MODULE = riot_wrappers_test_gnrc_pktbuf +BASELIBS += $(APPLICATION_RUST_MODULE).module +FEATURES_REQUIRED += rust_target + +# FIXME: That's not a good criterion, gnrc_pktbuf can well stand alone +USEMODULE += gnrc +USEMODULE += gnrc_pktbuf + +# thus we get `gnrc_pktbuf_is_empty` and `_is_sane` +CFLAGS += -DTEST_SUITES + +include $(RIOTBASE)/Makefile.include diff --git a/tests/gnrc-pktbuf/src/lib.rs b/tests/gnrc-pktbuf/src/lib.rs new file mode 100644 index 00000000..bc4666e4 --- /dev/null +++ b/tests/gnrc-pktbuf/src/lib.rs @@ -0,0 +1,57 @@ +#![no_std] + +use riot_wrappers::println; +use riot_wrappers::riot_main; + +riot_main!(main); + +#[track_caller] +fn check() { + assert!( + unsafe { riot_sys::gnrc_pktbuf_is_sane() }, + "GNRC packet buffer in unsound state" + ); +} + +#[track_caller] +fn check_empty() { + check(); + assert!( + unsafe { riot_sys::gnrc_pktbuf_is_empty() }, + "GNRC packet buffer should be empty at this point" + ); +} + +fn main() { + check_empty(); + + let snip1 = riot_wrappers::gnrc::pktbuf::Pktsnip::allocate( + 128, + riot_sys::gnrc_nettype_t_GNRC_NETTYPE_UNDEF, + ) + .unwrap(); + println!("Allocated pktsnip {:?}", snip1); + + check(); + + let snip2 = riot_wrappers::gnrc::pktbuf::Pktsnip::allocate_from( + &[1, 2, 3, 4], + riot_sys::gnrc_nettype_t_GNRC_NETTYPE_UNDEF, + ) + .unwrap(); + println!("Allocated another pktsnip {:?}", snip2); + let snip2 = snip2 + .add(10, riot_sys::gnrc_nettype_t_GNRC_NETTYPE_UNDEF) + .unwrap(); + println!("... and prefixed it with 10 bytes into {:?}", snip2); + + check(); + + drop(snip1); + drop(snip2); + println!("Dropped it"); + + check_empty(); + + println!("Tests completed."); +} diff --git a/tests/gnrc-pktbuf/tests/01-run.py b/tests/gnrc-pktbuf/tests/01-run.py new file mode 100755 index 00000000..ac26a3bd --- /dev/null +++ b/tests/gnrc-pktbuf/tests/01-run.py @@ -0,0 +1,10 @@ +#!/usr/bin/env python3 + +import sys +from testrunner import run + +def test(child): + child.expect_exact("Tests completed.") + +if __name__ == "__main__": + sys.exit(run(test)) From 7ae33f13e33bd1a15c8a0872aaa4b949ad19d3f2 Mon Sep 17 00:00:00 2001 From: chrysn Date: Sun, 15 Oct 2023 17:25:21 +0200 Subject: [PATCH 25/50] Move gnrc::pktbuf to gnrc_pktbuf The module can exist and be tested without all of GNRC --- src/gnrc.rs | 3 ++- src/{gnrc/pktbuf.rs => gnrc_pktbuf.rs} | 0 src/lib.rs | 3 +++ tests/gnrc-pktbuf/Makefile | 2 -- tests/gnrc-pktbuf/src/lib.rs | 4 ++-- 5 files changed, 7 insertions(+), 5 deletions(-) rename src/{gnrc/pktbuf.rs => gnrc_pktbuf.rs} (100%) diff --git a/src/gnrc.rs b/src/gnrc.rs index 5c0bca52..53839700 100644 --- a/src/gnrc.rs +++ b/src/gnrc.rs @@ -5,7 +5,8 @@ pub mod ipv6; pub mod netapi; pub mod netreg; -pub mod pktbuf; +#[deprecated(note = "moved to gnrc_pktbuf toplevel module")] +pub use crate::gnrc_pktbuf as pktbuf; use riot_sys::{gnrc_netif_iter, gnrc_netif_t}; diff --git a/src/gnrc/pktbuf.rs b/src/gnrc_pktbuf.rs similarity index 100% rename from src/gnrc/pktbuf.rs rename to src/gnrc_pktbuf.rs diff --git a/src/lib.rs b/src/lib.rs index 8c976e6a..5b2c745c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -105,6 +105,9 @@ pub mod thread; pub mod gcoap; #[cfg(riot_module_gnrc)] pub mod gnrc; +// Note that this can also exist without gnrc +#[cfg(riot_module_gnrc_pktbuf)] +pub mod gnrc_pktbuf; #[cfg(riot_module_gnrc)] pub mod gnrc_util; #[cfg(riot_module_periph_i2c)] diff --git a/tests/gnrc-pktbuf/Makefile b/tests/gnrc-pktbuf/Makefile index d31c9f22..36095a97 100644 --- a/tests/gnrc-pktbuf/Makefile +++ b/tests/gnrc-pktbuf/Makefile @@ -4,8 +4,6 @@ APPLICATION_RUST_MODULE = riot_wrappers_test_gnrc_pktbuf BASELIBS += $(APPLICATION_RUST_MODULE).module FEATURES_REQUIRED += rust_target -# FIXME: That's not a good criterion, gnrc_pktbuf can well stand alone -USEMODULE += gnrc USEMODULE += gnrc_pktbuf # thus we get `gnrc_pktbuf_is_empty` and `_is_sane` diff --git a/tests/gnrc-pktbuf/src/lib.rs b/tests/gnrc-pktbuf/src/lib.rs index bc4666e4..d2a81e3f 100644 --- a/tests/gnrc-pktbuf/src/lib.rs +++ b/tests/gnrc-pktbuf/src/lib.rs @@ -25,7 +25,7 @@ fn check_empty() { fn main() { check_empty(); - let snip1 = riot_wrappers::gnrc::pktbuf::Pktsnip::allocate( + let snip1 = riot_wrappers::gnrc_pktbuf::Pktsnip::allocate( 128, riot_sys::gnrc_nettype_t_GNRC_NETTYPE_UNDEF, ) @@ -34,7 +34,7 @@ fn main() { check(); - let snip2 = riot_wrappers::gnrc::pktbuf::Pktsnip::allocate_from( + let snip2 = riot_wrappers::gnrc_pktbuf::Pktsnip::allocate_from( &[1, 2, 3, 4], riot_sys::gnrc_nettype_t_GNRC_NETTYPE_UNDEF, ) From dfd7a89412de98015bc635545425511ca114b409 Mon Sep 17 00:00:00 2001 From: chrysn Date: Sun, 15 Oct 2023 18:05:43 +0200 Subject: [PATCH 26/50] tests/gnrc_pktbuf: Try switching between shared and writable --- tests/gnrc-pktbuf/src/lib.rs | 41 ++++++++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/tests/gnrc-pktbuf/src/lib.rs b/tests/gnrc-pktbuf/src/lib.rs index d2a81e3f..64ad97b5 100644 --- a/tests/gnrc-pktbuf/src/lib.rs +++ b/tests/gnrc-pktbuf/src/lib.rs @@ -3,6 +3,8 @@ use riot_wrappers::println; use riot_wrappers::riot_main; +use riot_wrappers::gnrc_pktbuf::{Pktsnip, Shared}; + riot_main!(main); #[track_caller] @@ -25,20 +27,13 @@ fn check_empty() { fn main() { check_empty(); - let snip1 = riot_wrappers::gnrc_pktbuf::Pktsnip::allocate( - 128, - riot_sys::gnrc_nettype_t_GNRC_NETTYPE_UNDEF, - ) - .unwrap(); + let snip1 = Pktsnip::allocate(128, riot_sys::gnrc_nettype_t_GNRC_NETTYPE_UNDEF).unwrap(); println!("Allocated pktsnip {:?}", snip1); check(); - let snip2 = riot_wrappers::gnrc_pktbuf::Pktsnip::allocate_from( - &[1, 2, 3, 4], - riot_sys::gnrc_nettype_t_GNRC_NETTYPE_UNDEF, - ) - .unwrap(); + let snip2 = + Pktsnip::allocate_from(&[1, 2, 3, 4], riot_sys::gnrc_nettype_t_GNRC_NETTYPE_UNDEF).unwrap(); println!("Allocated another pktsnip {:?}", snip2); let snip2 = snip2 .add(10, riot_sys::gnrc_nettype_t_GNRC_NETTYPE_UNDEF) @@ -47,8 +42,32 @@ fn main() { check(); + let snip2a: Pktsnip = snip2.into(); + let snip2b = snip2a.clone(); + let snip2b = snip2b + .add(10, riot_sys::gnrc_nettype_t_GNRC_NETTYPE_UNDEF) + .unwrap(); + println!( + "We have two of them now, the second one with an extension: {:?}, {:?}", + snip2a, snip2b + ); + + println!( + "Making the second writable should not change anything about memory usage (not tested)..." + ); + let snip2b: Pktsnip = snip2b.into(); // and the method isn't even available unless we + // make it shared in the first place + let snip2b = snip2b.start_write().unwrap(); + println!("... whereas making the first writable entails copying"); + let snip2a = snip2a.start_write().unwrap(); + println!( + "In the end, they still look the same: {:?}, {:?}", + snip2a, snip2b + ); + drop(snip1); - drop(snip2); + drop(snip2a); + drop(snip2b); println!("Dropped it"); check_empty(); From 438a346a952af855ac11cbb54f637f7f4c9b447d Mon Sep 17 00:00:00 2001 From: chrysn Date: Thu, 23 Nov 2023 21:55:16 +0100 Subject: [PATCH 27/50] random: Add module This provides an RNG similar to rand_core's OsRng, provided suitable RIOT modules are enabled. --- Cargo.toml | 1 + src/lib.rs | 2 ++ src/random.rs | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 69 insertions(+) create mode 100644 src/random.rs diff --git a/Cargo.toml b/Cargo.toml index 9cca74d9..d1ec0417 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -30,6 +30,7 @@ bare-metal = "1" cstr = "^0.2.11" heapless = "^0.7" +rand_core_06 = { package = "rand_core", version = "^0.6" } # For nimble UUID parsing and some debug implementations hex = { version = "^0.4.3", default-features = false } diff --git a/src/lib.rs b/src/lib.rs index f7fc829d..f922a16b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -121,6 +121,8 @@ pub mod gnrc_util; pub mod i2c; #[cfg(riot_module_core_msg)] pub mod msg; +#[cfg(riot_module_random)] +pub mod random; #[cfg(riot_module_periph_spi)] pub mod spi; diff --git a/src/random.rs b/src/random.rs new file mode 100644 index 00000000..7824022d --- /dev/null +++ b/src/random.rs @@ -0,0 +1,66 @@ +//! RIOT's system random number generator + +/// The global random number generator available in RIOT +/// +/// Actual functionality is available through its implementation of [rand_core_06::RngCore]. It +/// implements [rand_core_06::CryptoRng] if the RIOT module `prng_shaxprng` is present. (If more +/// CSPRNGs are implemented in RIOT, the list around this implementation needs to be extended). +/// +/// Note that this *is* copy (unlike what RngCore recommends) -- because the state is not in here +/// but global (as is their own OsRng). +#[derive(Copy, Clone, Debug)] +pub struct Random(()); + +impl Random { + /// Access the system random number generator + #[cfg(riot_module_auto_init_random)] + pub fn new() -> Self { + Random(()) + } + + /// Seed and start the random number generator + /// + /// While technically not unsound, this is marked unsafe as it may overwrite existing good RNG + /// state. + /// + /// This is not going through the the [rand_core_06::SeedableRng] trait because that would + /// require per-RNG seedability. + pub unsafe fn new_with_seed(seed: u32) -> Self { + riot_sys::random_init(seed); + Random(()) + } +} + +#[cfg(riot_module_auto_init_random)] +impl Default for Random { + fn default() -> Self { + Self::new() + } +} + +impl rand_core_06::RngCore for Random { + fn next_u32(&mut self) -> u32 { + // unsafe: C API makes no requirements, and the type ensures it's seeded + unsafe { riot_sys::random_uint32() } + } + + fn next_u64(&mut self) -> u64 { + let mut result = core::mem::MaybeUninit::uninit(); + // unsafe: C API makes no requirements, and the type ensures it's seeded + unsafe { riot_sys::random_bytes(result.as_mut_ptr() as _, 8) }; + // unsafe: We had it written, and every state is inhabited + unsafe { result.assume_init() } + } + + fn fill_bytes(&mut self, dest: &mut [u8]) { + // unsafe: C API makes no requirements, and the type ensures it's seeded + unsafe { riot_sys::random_bytes(dest.as_mut_ptr() as _, dest.len() as _) }; + } + + fn try_fill_bytes(&mut self, dest: &mut [u8]) -> Result<(), rand_core_06::Error> { + Ok(self.fill_bytes(dest)) + } +} + +#[cfg(riot_module_prng_shaxprng)] +impl rand_core_06::CryptoRng for Random {} From eff6b7962851bfd1f8c994b4d821611f456b8b4a Mon Sep 17 00:00:00 2001 From: chrysn Date: Fri, 24 Nov 2023 11:26:04 +0100 Subject: [PATCH 28/50] CI: Run on main and on request in addition to pull requests --- .github/workflows/buildtest.yml | 5 +++-- .github/workflows/rustfmt.yml | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/.github/workflows/buildtest.yml b/.github/workflows/buildtest.yml index 1a9fb92a..053c791c 100644 --- a/.github/workflows/buildtest.yml +++ b/.github/workflows/buildtest.yml @@ -5,8 +5,9 @@ name: build-test on: pull_request: - branches: - - '*' + push: + branches: [main] + workflow_dispatch: jobs: build-test: diff --git a/.github/workflows/rustfmt.yml b/.github/workflows/rustfmt.yml index ee484079..cac8241f 100644 --- a/.github/workflows/rustfmt.yml +++ b/.github/workflows/rustfmt.yml @@ -2,8 +2,9 @@ name: rustfmt on: pull_request: - branches: - - '*' + push: + branches: [main] + workflow_dispatch: jobs: rustfmt: From 725d049cef8132659e8d6203c515983c5e90f781 Mon Sep 17 00:00:00 2001 From: chrysn Date: Fri, 24 Nov 2023 11:59:16 +0100 Subject: [PATCH 29/50] CI: Split update and build, run in parallel --- .github/workflows/buildtest.yml | 69 ++++++++++++++++++--------------- 1 file changed, 38 insertions(+), 31 deletions(-) diff --git a/.github/workflows/buildtest.yml b/.github/workflows/buildtest.yml index 053c791c..e316def3 100644 --- a/.github/workflows/buildtest.yml +++ b/.github/workflows/buildtest.yml @@ -10,7 +10,7 @@ on: workflow_dispatch: jobs: - build-test: + prepare: runs-on: ubuntu-latest container: riot/riotbuild steps: @@ -27,46 +27,53 @@ jobs: echo '[patch.crates-io]' >> .cargo/config.toml echo 'riot-wrappers = { path = "../", version = "*" }' >> .cargo/config.toml echo 'riot-sys = { git = "https://github.com/RIOT-OS/rust-riot-sys" }' >> .cargo/config.toml - - name: Build on selected platforms - # not going with a - # - # strategy: - # matrix: - # example: [examples/rust-hello-world examples/rust-gcoap tests/rust_minimal] - # - # setup here because really most of the stuff is the same, and the `cargo - # update` is much faster the second time (so a parallel execution may - # still be faster but uses 3x the resources) + - name: Pull cargo updates + # No sense in running this in parallel -- this will download the index + # and all relevant crates once, and after that, the run: | - export BOARDS='native sltb001a samr21-xpro stk3700' - DIRS='examples/rust-hello-world examples/rust-gcoap tests/rust_minimal' # It appears that there has to be output before :: commands really catch on - echo "Building ${DIRS} on ${BOARDS}" + echo "Pulling updates" echo "::echo ::on" - cd RIOT - for D in ${DIRS}; do - cd ${D} - echo "::group::Building ${D}" - cargo update -p riot-sys -p riot-wrappers --aggressive - cargo tree - make buildtest BUILDTEST_MAKE_REDIRECT='' - cd ../.. - echo "::endgroup::" + for MANIF in $(find RIOT -name Cargo.toml) + do + echo "::group::Updating ${MANIF}" + cargo update -p riot-sys -p riot-wrappers --aggressive --manifest-path $MANIF + cargo fetch --manifest-path $MANIF + cargo tree --manifest-path $MANIF + echo "::endgroup::" done - cd .. - echo "::echo ::off" + + build-examples: + needs: prepare + runs-on: ubuntu-latest + container: riot/riotbuild + strategy: + matrix: + example: [examples/rust-hello-world, examples/rust-gcoap, tests/rust_minimal] + board: [native, sltb001a, samr32-xpro, stk3700] + steps: + - name: Build on selected platforms + run: | + make buildtest BUILDTEST_MAKE_REDIRECT='' BOARD=${{ matrix.board }} -C RIOT/${{ matrix.example }} + + build-tests: + needs: prepare + runs-on: ubuntu-latest + container: riot/riotbuild + strategy: + matrix: + board: [native, sltb001a, samr21-xpro, stk3700] + steps: - name: Build and run tests run: | - export BOARDS='native sltb001a samr21-xpro stk3700' - DIRS=$(echo tests/*/) + DIRS=$(echo RIOT/tests/*/) export RIOTBASE=$(pwd)/RIOT # It appears that there has to be output before :: commands really catch on - echo "Building ${DIRS} on ${BOARDS}" + echo "Building ${DIRS} on ${{ matrix.board }}" echo "::echo ::on" for D in ${DIRS}; do - cd ${D} + pushd ${D} echo "::group::Building ${D}" - cargo update -p riot-sys -p riot-wrappers --aggressive make buildtest BUILDTEST_MAKE_REDIRECT='' echo "::endgroup::" if make test/available BOARD=native; then @@ -74,6 +81,6 @@ jobs: make all test BOARD=native echo "::endgroup::" fi - cd ../.. + popd done echo "::echo ::off" From 224f4858b2f2c7a69ad584f7b711f074be6518b7 Mon Sep 17 00:00:00 2001 From: chrysn Date: Fri, 24 Nov 2023 12:42:41 +0100 Subject: [PATCH 30/50] CI: Run everything in parallel The largest amount of time is the machine checkout -- 2 minutes. The 7 seconds of `cargo update` are dwarved by that, and running in parallel means we don't have to eat those 2 minutes twice. (Also, I couldn't figure out how to carry the output state of the `needs`'d preparations over to the actual working steps). --- .github/workflows/buildtest.yml | 51 +++++++++++++++++++++++++-------- 1 file changed, 39 insertions(+), 12 deletions(-) diff --git a/.github/workflows/buildtest.yml b/.github/workflows/buildtest.yml index e316def3..b77c8e26 100644 --- a/.github/workflows/buildtest.yml +++ b/.github/workflows/buildtest.yml @@ -10,10 +10,15 @@ on: workflow_dispatch: jobs: - prepare: + build-examples: runs-on: ubuntu-latest container: riot/riotbuild + strategy: + matrix: + example: [examples/rust-hello-world, examples/rust-gcoap, tests/rust_minimal] + board: [native, sltb001a, samr32-xpro, stk3700] steps: + # common steps start here - uses: actions/checkout@v3 - uses: actions/checkout@v3 with: @@ -29,7 +34,7 @@ jobs: echo 'riot-sys = { git = "https://github.com/RIOT-OS/rust-riot-sys" }' >> .cargo/config.toml - name: Pull cargo updates # No sense in running this in parallel -- this will download the index - # and all relevant crates once, and after that, the + # and all relevant crates once, and after that, just make some notes in Cargo.lock run: | # It appears that there has to be output before :: commands really catch on echo "Pulling updates" @@ -42,28 +47,50 @@ jobs: cargo tree --manifest-path $MANIF echo "::endgroup::" done + # common steps end here - build-examples: - needs: prepare - runs-on: ubuntu-latest - container: riot/riotbuild - strategy: - matrix: - example: [examples/rust-hello-world, examples/rust-gcoap, tests/rust_minimal] - board: [native, sltb001a, samr32-xpro, stk3700] - steps: - name: Build on selected platforms run: | make buildtest BUILDTEST_MAKE_REDIRECT='' BOARD=${{ matrix.board }} -C RIOT/${{ matrix.example }} build-tests: - needs: prepare runs-on: ubuntu-latest container: riot/riotbuild strategy: matrix: board: [native, sltb001a, samr21-xpro, stk3700] steps: + # common steps start here + - uses: actions/checkout@v3 + - uses: actions/checkout@v3 + with: + repository: RIOT-OS/RIOT + path: RIOT + - name: Patch .cargo/config.toml to use current checkout + run: | + cd RIOT + rm -f .cargo/config.toml + mkdir -p .cargo # Keep working if RIOT ever decides it doesn't need overrides any more + echo '[patch.crates-io]' >> .cargo/config.toml + echo 'riot-wrappers = { path = "../", version = "*" }' >> .cargo/config.toml + echo 'riot-sys = { git = "https://github.com/RIOT-OS/rust-riot-sys" }' >> .cargo/config.toml + - name: Pull cargo updates + # No sense in running this in parallel -- this will download the index + # and all relevant crates once, and after that, just make some notes in Cargo.lock + run: | + # It appears that there has to be output before :: commands really catch on + echo "Pulling updates" + echo "::echo ::on" + for MANIF in $(find RIOT -name Cargo.toml) + do + echo "::group::Updating ${MANIF}" + cargo update -p riot-sys -p riot-wrappers --aggressive --manifest-path $MANIF + cargo fetch --manifest-path $MANIF + cargo tree --manifest-path $MANIF + echo "::endgroup::" + done + # common steps end here + - name: Build and run tests run: | DIRS=$(echo RIOT/tests/*/) From 80f2b3ef08ea07b7208e1767bd0e20e0e35dcc83 Mon Sep 17 00:00:00 2001 From: chrysn Date: Fri, 24 Nov 2023 14:52:47 +0100 Subject: [PATCH 31/50] tests: Add random test --- tests/random/Cargo.toml | 17 ++++++++ tests/random/Makefile | 14 +++++++ tests/random/src/lib.rs | 77 ++++++++++++++++++++++++++++++++++++ tests/random/tests/01-run.py | 11 ++++++ 4 files changed, 119 insertions(+) create mode 100644 tests/random/Cargo.toml create mode 100644 tests/random/Makefile create mode 100644 tests/random/src/lib.rs create mode 100755 tests/random/tests/01-run.py diff --git a/tests/random/Cargo.toml b/tests/random/Cargo.toml new file mode 100644 index 00000000..f7ac18c8 --- /dev/null +++ b/tests/random/Cargo.toml @@ -0,0 +1,17 @@ +[package] +name = "riot-wrappers-test-random" +version = "0.1.0" +authors = ["Christian Amsüss "] +edition = "2021" +publish = false + +[lib] +crate-type = ["staticlib"] + +[profile.release] +panic = "abort" + +[dependencies] +riot-wrappers = { version = "*", features = [ "set_panic_handler" ] } +rand_core = "0.6" +rngcheck = "0.1.1" diff --git a/tests/random/Makefile b/tests/random/Makefile new file mode 100644 index 00000000..e8fe3e7d --- /dev/null +++ b/tests/random/Makefile @@ -0,0 +1,14 @@ +# name of your application +APPLICATION = riot-wrappers-test-random +BOARD ?= native +APPLICATION_RUST_MODULE = riot_wrappers_test_random +BASELIBS += $(APPLICATION_RUST_MODULE).module +FEATURES_REQUIRED += rust_target + +USEMODULE += random +# So that the RNG can be Default-constructed +FEATURES_REQUIRED += periph_hwrng +# So that the RNG also satisfies the CryptoRng requirements +USEMODULE += prng_sha256prng + +include $(RIOTBASE)/Makefile.include diff --git a/tests/random/src/lib.rs b/tests/random/src/lib.rs new file mode 100644 index 00000000..a6d0b52d --- /dev/null +++ b/tests/random/src/lib.rs @@ -0,0 +1,77 @@ +#![no_std] + +use riot_wrappers::println; +use riot_wrappers::random::Random; +use riot_wrappers::riot_main; + +riot_main!(main); + +fn check_csrng(mut rng: impl rand_core::CryptoRng + rand_core::RngCore) { + use rngcheck::{helpers::*, nist::*}; + + struct BitIter<'a, R: rand_core::RngCore> { + rng: &'a mut R, + remaining: usize, + buffer: u32, + buffered: u8, + } + + impl<'a, R: rand_core::RngCore> BitIter<'a, R> { + fn new(rng: &'a mut R, items: usize) -> Self { + Self { + rng, + remaining: items, + buffer: 0, + buffered: 0, + } + } + } + impl<'a, R: rand_core::RngCore> Iterator for BitIter<'a, R> { + type Item = bool; + fn next(&mut self) -> Option { + if self.remaining == 0 { + return None; + } + + self.remaining -= 1; + + if self.buffered == 0 { + self.buffer = self.rng.next_u32(); + self.buffered = 32; + } + let result = self.buffer & 1 != 0; + self.buffer >>= 1; + self.buffered -= 1; + Some(result) + } + } + + // 1 megabit; enough to complete on an nRF52840 within the test timeout + let count = 1000000; + + println!( + "Monobit stat: {}", + nist_freq_monobit(BitIter::new(&mut rng, count)).unwrap() + ); + // To be fair, I have no clue whether 16 is a good number here. + // + // These are all reporting NaN due to https://github.com/ryankurte/rngcheck/issues/1 -- but + // then again, we're mainly doing these so that the RNG runs for a bit. + println!( + "Frequency block stat: {}", + nist_freq_block(BitIter::new(&mut rng, count), 16).unwrap() + ); + + println!("Generating 10 more random numbers"); + for _ in 0..10 { + println!("{}", rng.next_u32()); + } + + println!("Done"); +} + +fn main() { + // works because we have periph_hwrng + let rng: Random = Default::default(); + check_csrng(rng); +} diff --git a/tests/random/tests/01-run.py b/tests/random/tests/01-run.py new file mode 100755 index 00000000..7da7f38f --- /dev/null +++ b/tests/random/tests/01-run.py @@ -0,0 +1,11 @@ +#!/usr/bin/env python3 + +import os +import sys +from testrunner import run + +def test(child): + child.expect("Done") + +if __name__ == "__main__": + sys.exit(run(test)) From 6926bd1cb78ba13accb767824ee8f9bfdf26524b Mon Sep 17 00:00:00 2001 From: chrysn Date: Fri, 24 Nov 2023 12:55:02 +0100 Subject: [PATCH 32/50] CI: Fix minor errors --- .github/workflows/buildtest.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/buildtest.yml b/.github/workflows/buildtest.yml index b77c8e26..eae0fc46 100644 --- a/.github/workflows/buildtest.yml +++ b/.github/workflows/buildtest.yml @@ -16,7 +16,7 @@ jobs: strategy: matrix: example: [examples/rust-hello-world, examples/rust-gcoap, tests/rust_minimal] - board: [native, sltb001a, samr32-xpro, stk3700] + board: [native, sltb001a, samr21-xpro, stk3700] steps: # common steps start here - uses: actions/checkout@v3 @@ -99,7 +99,7 @@ jobs: echo "Building ${DIRS} on ${{ matrix.board }}" echo "::echo ::on" for D in ${DIRS}; do - pushd ${D} + cd ${D} echo "::group::Building ${D}" make buildtest BUILDTEST_MAKE_REDIRECT='' echo "::endgroup::" @@ -108,6 +108,6 @@ jobs: make all test BOARD=native echo "::endgroup::" fi - popd + cd - done echo "::echo ::off" From 394a02522f2b73be9718d012a5c414ed37059fcb Mon Sep 17 00:00:00 2001 From: chrysn Date: Fri, 24 Nov 2023 13:35:39 +0100 Subject: [PATCH 33/50] CI: Adjust to not all tests sitting in tests/* any more --- .github/workflows/buildtest.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/buildtest.yml b/.github/workflows/buildtest.yml index eae0fc46..a67a47b1 100644 --- a/.github/workflows/buildtest.yml +++ b/.github/workflows/buildtest.yml @@ -93,7 +93,7 @@ jobs: - name: Build and run tests run: | - DIRS=$(echo RIOT/tests/*/) + DIRS=$(find RIOT/tests -name Makefile | xargs -n1 dirname) export RIOTBASE=$(pwd)/RIOT # It appears that there has to be output before :: commands really catch on echo "Building ${DIRS} on ${{ matrix.board }}" From 1a31e0b925ce75fc0c6d1968878940c6564015a9 Mon Sep 17 00:00:00 2001 From: chrysn Date: Fri, 24 Nov 2023 13:42:22 +0100 Subject: [PATCH 34/50] CI fix: Run on the selected board instead of all boards --- .github/workflows/buildtest.yml | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/.github/workflows/buildtest.yml b/.github/workflows/buildtest.yml index a67a47b1..b2fe3b15 100644 --- a/.github/workflows/buildtest.yml +++ b/.github/workflows/buildtest.yml @@ -49,9 +49,9 @@ jobs: done # common steps end here - - name: Build on selected platforms + - name: Build the example run: | - make buildtest BUILDTEST_MAKE_REDIRECT='' BOARD=${{ matrix.board }} -C RIOT/${{ matrix.example }} + make all BOARD=${{ matrix.board }} -C RIOT/${{ matrix.example }} build-tests: runs-on: ubuntu-latest @@ -101,9 +101,11 @@ jobs: for D in ${DIRS}; do cd ${D} echo "::group::Building ${D}" - make buildtest BUILDTEST_MAKE_REDIRECT='' + # By running buildtest instead of all, we just skip boards that don't + # satisfy some requirements for some test + make buildtest BUILDTEST_MAKE_REDIRECT='' BOARDS=${{ matrix.board }} echo "::endgroup::" - if make test/available BOARD=native; then + if make test/available BOARD=native && [ "native" = "${{ matrix.board }}" ]; then echo "::group::Testing ${D}" make all test BOARD=native echo "::endgroup::" From 8cdc14eb4ebdedab4144075afa64d18667d3aaf6 Mon Sep 17 00:00:00 2001 From: chrysn Date: Fri, 24 Nov 2023 13:50:49 +0100 Subject: [PATCH 35/50] CI: continue on error in example tests --- .github/workflows/buildtest.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/buildtest.yml b/.github/workflows/buildtest.yml index b2fe3b15..586d9959 100644 --- a/.github/workflows/buildtest.yml +++ b/.github/workflows/buildtest.yml @@ -13,6 +13,8 @@ jobs: build-examples: runs-on: ubuntu-latest container: riot/riotbuild + # Best would be "continue until there are errors from different examples *and* different boards" + continue-on-error: true strategy: matrix: example: [examples/rust-hello-world, examples/rust-gcoap, tests/rust_minimal] From 154222077f5726d8f21482b58d701e3a9cb804b0 Mon Sep 17 00:00:00 2001 From: chrysn Date: Fri, 24 Nov 2023 17:20:03 +0100 Subject: [PATCH 36/50] CI: Fix make invocation error that caused tests to fail when a board lacks their prerequisites --- .github/workflows/buildtest.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/buildtest.yml b/.github/workflows/buildtest.yml index 586d9959..7ffe9d99 100644 --- a/.github/workflows/buildtest.yml +++ b/.github/workflows/buildtest.yml @@ -105,7 +105,7 @@ jobs: echo "::group::Building ${D}" # By running buildtest instead of all, we just skip boards that don't # satisfy some requirements for some test - make buildtest BUILDTEST_MAKE_REDIRECT='' BOARDS=${{ matrix.board }} + BOARDS=${{ matrix.board }} make buildtest BUILDTEST_MAKE_REDIRECT='' echo "::endgroup::" if make test/available BOARD=native && [ "native" = "${{ matrix.board }}" ]; then echo "::group::Testing ${D}" From d633e3c812a8bbe6585a4a5d2f04d9c3b343edb5 Mon Sep 17 00:00:00 2001 From: chrysn Date: Fri, 24 Nov 2023 17:46:59 +0100 Subject: [PATCH 37/50] CI: Explicitly check for board support ... because the test running clause would not test on its own and fail on missing feature requirements --- .github/workflows/buildtest.yml | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/.github/workflows/buildtest.yml b/.github/workflows/buildtest.yml index 7ffe9d99..1c07ac30 100644 --- a/.github/workflows/buildtest.yml +++ b/.github/workflows/buildtest.yml @@ -101,11 +101,10 @@ jobs: echo "Building ${DIRS} on ${{ matrix.board }}" echo "::echo ::on" for D in ${DIRS}; do - cd ${D} echo "::group::Building ${D}" - # By running buildtest instead of all, we just skip boards that don't - # satisfy some requirements for some test - BOARDS=${{ matrix.board }} make buildtest BUILDTEST_MAKE_REDIRECT='' + cd ${D} + BOARDS=${{ matrix.board }} make info-boards-supported | grep -q . || ( cd -; continue ) + BOARD=${{ matrix.board }} make all echo "::endgroup::" if make test/available BOARD=native && [ "native" = "${{ matrix.board }}" ]; then echo "::group::Testing ${D}" From aaedea9111b7073c4a924136011cfb69ce459a02 Mon Sep 17 00:00:00 2001 From: chrysn Date: Fri, 24 Nov 2023 21:11:53 +0100 Subject: [PATCH 38/50] CI: Overhaul debug output At least with the current GitHub versions, ::echo ::on doesn't do anything. --- .github/workflows/buildtest.yml | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/.github/workflows/buildtest.yml b/.github/workflows/buildtest.yml index 1c07ac30..244a3837 100644 --- a/.github/workflows/buildtest.yml +++ b/.github/workflows/buildtest.yml @@ -21,13 +21,16 @@ jobs: board: [native, sltb001a, samr21-xpro, stk3700] steps: # common steps start here - - uses: actions/checkout@v3 - - uses: actions/checkout@v3 + - name: Check out riot-wrappers + uses: actions/checkout@v3 + - name: Check out RIOT + uses: actions/checkout@v3 with: repository: RIOT-OS/RIOT path: RIOT - name: Patch .cargo/config.toml to use current checkout run: | + set -x cd RIOT rm -f .cargo/config.toml mkdir -p .cargo # Keep working if RIOT ever decides it doesn't need overrides any more @@ -38,9 +41,7 @@ jobs: # No sense in running this in parallel -- this will download the index # and all relevant crates once, and after that, just make some notes in Cargo.lock run: | - # It appears that there has to be output before :: commands really catch on - echo "Pulling updates" - echo "::echo ::on" + set -x for MANIF in $(find RIOT -name Cargo.toml) do echo "::group::Updating ${MANIF}" @@ -63,13 +64,16 @@ jobs: board: [native, sltb001a, samr21-xpro, stk3700] steps: # common steps start here - - uses: actions/checkout@v3 - - uses: actions/checkout@v3 + - name: Check out riot-wrappers + uses: actions/checkout@v3 + - name: Check out RIOT + uses: actions/checkout@v3 with: repository: RIOT-OS/RIOT path: RIOT - name: Patch .cargo/config.toml to use current checkout run: | + set -x cd RIOT rm -f .cargo/config.toml mkdir -p .cargo # Keep working if RIOT ever decides it doesn't need overrides any more @@ -80,9 +84,7 @@ jobs: # No sense in running this in parallel -- this will download the index # and all relevant crates once, and after that, just make some notes in Cargo.lock run: | - # It appears that there has to be output before :: commands really catch on - echo "Pulling updates" - echo "::echo ::on" + set -x for MANIF in $(find RIOT -name Cargo.toml) do echo "::group::Updating ${MANIF}" @@ -95,15 +97,13 @@ jobs: - name: Build and run tests run: | + set -x DIRS=$(find RIOT/tests -name Makefile | xargs -n1 dirname) export RIOTBASE=$(pwd)/RIOT - # It appears that there has to be output before :: commands really catch on - echo "Building ${DIRS} on ${{ matrix.board }}" - echo "::echo ::on" for D in ${DIRS}; do echo "::group::Building ${D}" cd ${D} - BOARDS=${{ matrix.board }} make info-boards-supported | grep -q . || ( cd -; continue ) + BOARDS=${{ matrix.board }} make info-boards-supported | grep -q . || ( echo "Board is not supported for this test"; cd -; continue ) BOARD=${{ matrix.board }} make all echo "::endgroup::" if make test/available BOARD=native && [ "native" = "${{ matrix.board }}" ]; then From 0965b3e41f7eb594ddca482ad07b008c4fc4e7f0 Mon Sep 17 00:00:00 2001 From: chrysn Date: Sat, 25 Nov 2023 12:45:38 +0100 Subject: [PATCH 39/50] CI: Avoid fancy sh things that go wrong --- .github/workflows/buildtest.yml | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/.github/workflows/buildtest.yml b/.github/workflows/buildtest.yml index 244a3837..f4bbdc89 100644 --- a/.github/workflows/buildtest.yml +++ b/.github/workflows/buildtest.yml @@ -103,14 +103,20 @@ jobs: for D in ${DIRS}; do echo "::group::Building ${D}" cd ${D} - BOARDS=${{ matrix.board }} make info-boards-supported | grep -q . || ( echo "Board is not supported for this test"; cd -; continue ) - BOARD=${{ matrix.board }} make all - echo "::endgroup::" - if make test/available BOARD=native && [ "native" = "${{ matrix.board }}" ]; then - echo "::group::Testing ${D}" - make all test BOARD=native - echo "::endgroup::" + if BOARDS=${{ matrix.board }} make info-boards-supported | grep -q . + then + BOARD=${{ matrix.board }} make all + + if [ "native" = "${{ matrix.board }}" ] && make test/available BOARD=native + then + echo "::group::Testing ${D}" + make all test BOARD=native + echo "::endgroup::" + fi + else + echo "Board is not supported for this test, skipping." fi cd - + echo "::endgroup::" done echo "::echo ::off" From 4f22e83dbef7c6f0d63f52c7482b73e040e0f5e9 Mon Sep 17 00:00:00 2001 From: chrysn Date: Sat, 25 Nov 2023 14:06:02 +0100 Subject: [PATCH 40/50] random/tests: Small improvements * If any of the tests fail, panic_format ensures the output can be understood easily. * Some code was sent upstream to [1], a note ensures the local code can be removed when ready. * Reducing the count of printed numbers to ensure that the default USB output buffer displays all relevant information and not just literally random stuff. [1]: https://github.com/ryankurte/rngcheck/pull/3 --- tests/random/Cargo.toml | 2 +- tests/random/src/lib.rs | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/random/Cargo.toml b/tests/random/Cargo.toml index f7ac18c8..39da138e 100644 --- a/tests/random/Cargo.toml +++ b/tests/random/Cargo.toml @@ -12,6 +12,6 @@ crate-type = ["staticlib"] panic = "abort" [dependencies] -riot-wrappers = { version = "*", features = [ "set_panic_handler" ] } +riot-wrappers = { version = "*", features = [ "set_panic_handler", "panic_handler_format" ] } rand_core = "0.6" rngcheck = "0.1.1" diff --git a/tests/random/src/lib.rs b/tests/random/src/lib.rs index a6d0b52d..704b3dc5 100644 --- a/tests/random/src/lib.rs +++ b/tests/random/src/lib.rs @@ -9,6 +9,7 @@ riot_main!(main); fn check_csrng(mut rng: impl rand_core::CryptoRng + rand_core::RngCore) { use rngcheck::{helpers::*, nist::*}; + // This is also in https://github.com/ryankurte/rngcheck/pull/3 struct BitIter<'a, R: rand_core::RngCore> { rng: &'a mut R, remaining: usize, @@ -62,8 +63,8 @@ fn check_csrng(mut rng: impl rand_core::CryptoRng + rand_core::RngCore) { nist_freq_block(BitIter::new(&mut rng, count), 16).unwrap() ); - println!("Generating 10 more random numbers"); - for _ in 0..10 { + println!("Generating 3 more random numbers"); + for _ in 0..3 { println!("{}", rng.next_u32()); } From aa5e88990452aff679cd06704af67fe464596f96 Mon Sep 17 00:00:00 2001 From: chrysn Date: Sun, 26 Nov 2023 02:05:03 +0100 Subject: [PATCH 41/50] CI: Replicate a special-casing of RIOT_CI_BUILD (But not enabling RIOT_CI_BUILD b/c we're not inside a Murdock run, and who knows what else that flag enables.) --- .github/workflows/buildtest.yml | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/.github/workflows/buildtest.yml b/.github/workflows/buildtest.yml index f4bbdc89..84cfe0c3 100644 --- a/.github/workflows/buildtest.yml +++ b/.github/workflows/buildtest.yml @@ -105,7 +105,17 @@ jobs: cd ${D} if BOARDS=${{ matrix.board }} make info-boards-supported | grep -q . then - BOARD=${{ matrix.board }} make all + # RIOTNOLINK is a workaround for boards with insufficient memory + # showing as supported but still being known not to be buildable. + # Other CI works around by having RIOT_CI_BUILD set RIOTNOLINK if + # the board is known to not have enough memory from + # BOARD_INSUFFICIENT_MEMORY, but that information is not piped out. + # + # Until a better workaround is found, no boards are linked, and if + # a board does exceed its memory due to a Rust change, that will + # only be spotted when the Rust crate is updated in RIOT and a full + # test with the precise criterion is run. + BOARD=${{ matrix.board }} make all RIOTNOLINK=1 if [ "native" = "${{ matrix.board }}" ] && make test/available BOARD=native then From e8c6c6395e937f7fe0886977a10cfeaed4c57543 Mon Sep 17 00:00:00 2001 From: chrysn Date: Sun, 26 Nov 2023 11:27:39 +0100 Subject: [PATCH 42/50] CI: Skip LVGL test which is currently broken Workaround-For: https://github.com/RIOT-OS/RIOT/issues/20110 --- .github/workflows/buildtest.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/buildtest.yml b/.github/workflows/buildtest.yml index 84cfe0c3..58a6baf4 100644 --- a/.github/workflows/buildtest.yml +++ b/.github/workflows/buildtest.yml @@ -98,7 +98,8 @@ jobs: - name: Build and run tests run: | set -x - DIRS=$(find RIOT/tests -name Makefile | xargs -n1 dirname) + # Removing tests/pkg/lvgl due to https://github.com/RIOT-OS/RIOT/issues/20110 + DIRS=$(find RIOT/tests -name Makefile | grep -v tests/pkg/lvgl | xargs -n1 dirname) export RIOTBASE=$(pwd)/RIOT for D in ${DIRS}; do echo "::group::Building ${D}" From 1cef1c743aa4eb6e8e328acfdf47350c1909cdb9 Mon Sep 17 00:00:00 2001 From: chrysn Date: Sun, 26 Nov 2023 12:51:10 +0100 Subject: [PATCH 43/50] CI: Clean after tests to avoid filling up disk --- .github/workflows/buildtest.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/buildtest.yml b/.github/workflows/buildtest.yml index 58a6baf4..752ed215 100644 --- a/.github/workflows/buildtest.yml +++ b/.github/workflows/buildtest.yml @@ -124,6 +124,7 @@ jobs: make all test BOARD=native echo "::endgroup::" fi + BOARD=${{ matrix.board }} make clean else echo "Board is not supported for this test, skipping." fi From c7e352fd056e8d10794171d9f9b6b118f1955c40 Mon Sep 17 00:00:00 2001 From: chrysn Date: Fri, 1 Dec 2023 19:14:50 +0100 Subject: [PATCH 44/50] CI: Fix how list of tests is obtained Some Makefile are for tools inside tests --- .github/workflows/buildtest.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/buildtest.yml b/.github/workflows/buildtest.yml index 752ed215..7dba0380 100644 --- a/.github/workflows/buildtest.yml +++ b/.github/workflows/buildtest.yml @@ -99,7 +99,7 @@ jobs: run: | set -x # Removing tests/pkg/lvgl due to https://github.com/RIOT-OS/RIOT/issues/20110 - DIRS=$(find RIOT/tests -name Makefile | grep -v tests/pkg/lvgl | xargs -n1 dirname) + DIRS=$(make -f makefiles/app_dirs.inc.mk info-applications | grep -v tests/pkg/lvgl) export RIOTBASE=$(pwd)/RIOT for D in ${DIRS}; do echo "::group::Building ${D}" From 333c18862f5542bf949a37bf6a851646b5d3830d Mon Sep 17 00:00:00 2001 From: chrysn Date: Fri, 1 Dec 2023 19:17:56 +0100 Subject: [PATCH 45/50] CI: Trim list of executed tests --- .github/workflows/buildtest.yml | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/.github/workflows/buildtest.yml b/.github/workflows/buildtest.yml index 7dba0380..f2fd0d93 100644 --- a/.github/workflows/buildtest.yml +++ b/.github/workflows/buildtest.yml @@ -98,8 +98,17 @@ jobs: - name: Build and run tests run: | set -x + cd RIOT # Removing tests/pkg/lvgl due to https://github.com/RIOT-OS/RIOT/issues/20110 - DIRS=$(make -f makefiles/app_dirs.inc.mk info-applications | grep -v tests/pkg/lvgl) + # + # Skipping examples because ... not sure, did it that way before + # https://github.com/RIOT-OS/rust-riot-wrappers/pull/68, and examples + # are built extra + # + # Skipping peripheral and driver tests because we don't implement those + # with wrappers; the valuable ones are those like saul where there are + # modules that use Rust. + DIRS=$(make -f makefiles/app_dirs.inc.mk info-applications | grep -v tests/pkg/lvgl |grep -v examples |grep -v periph |grep -v driver) export RIOTBASE=$(pwd)/RIOT for D in ${DIRS}; do echo "::group::Building ${D}" From 2883a28fe8673f02067784fe2c88da3b356756fc Mon Sep 17 00:00:00 2001 From: chrysn Date: Fri, 1 Dec 2023 19:44:19 +0100 Subject: [PATCH 46/50] CI: Better names for components --- .github/workflows/buildtest.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/buildtest.yml b/.github/workflows/buildtest.yml index f2fd0d93..d08c91ae 100644 --- a/.github/workflows/buildtest.yml +++ b/.github/workflows/buildtest.yml @@ -10,7 +10,7 @@ on: workflow_dispatch: jobs: - build-examples: + quick-examples: runs-on: ubuntu-latest container: riot/riotbuild # Best would be "continue until there are errors from different examples *and* different boards" @@ -56,7 +56,7 @@ jobs: run: | make all BOARD=${{ matrix.board }} -C RIOT/${{ matrix.example }} - build-tests: + most-tests: runs-on: ubuntu-latest container: riot/riotbuild strategy: From 34fc0c5ff9832fcb328ab8c4bfa2162d1b53ef6b Mon Sep 17 00:00:00 2001 From: chrysn Date: Fri, 1 Dec 2023 19:48:07 +0100 Subject: [PATCH 47/50] CI: Unify workflows to show as a single item in the actions list --- .github/workflows/rustfmt.yml | 18 ------------------ .github/workflows/{buildtest.yml => test.yml} | 12 +++++++++++- 2 files changed, 11 insertions(+), 19 deletions(-) delete mode 100644 .github/workflows/rustfmt.yml rename .github/workflows/{buildtest.yml => test.yml} (96%) diff --git a/.github/workflows/rustfmt.yml b/.github/workflows/rustfmt.yml deleted file mode 100644 index cac8241f..00000000 --- a/.github/workflows/rustfmt.yml +++ /dev/null @@ -1,18 +0,0 @@ -name: rustfmt - -on: - pull_request: - push: - branches: [main] - workflow_dispatch: - -jobs: - rustfmt: - runs-on: ubuntu-latest - container: rustlang/rust:nightly - steps: - - uses: actions/checkout@v3 - with: - fetch-depth: 0 - - name: Run cargo-fmt - run: cargo fmt --check diff --git a/.github/workflows/buildtest.yml b/.github/workflows/test.yml similarity index 96% rename from .github/workflows/buildtest.yml rename to .github/workflows/test.yml index d08c91ae..29d37f0a 100644 --- a/.github/workflows/buildtest.yml +++ b/.github/workflows/test.yml @@ -1,7 +1,7 @@ # Ideally this should be replaced with a call out to Murdock; until that is # practical, building representative examples. -name: build-test +name: test on: pull_request: @@ -141,3 +141,13 @@ jobs: echo "::endgroup::" done echo "::echo ::off" + + rustfmt: + runs-on: ubuntu-latest + container: rustlang/rust:nightly + steps: + - uses: actions/checkout@v3 + with: + fetch-depth: 0 + - name: Run cargo-fmt + run: cargo fmt --check From 633d87c47e8dbd54812e67086f57e09204549a64 Mon Sep 17 00:00:00 2001 From: chrysn Date: Fri, 1 Dec 2023 19:54:06 +0100 Subject: [PATCH 48/50] CI: Fix cd screwup that made almost-no-tests run --- .github/workflows/test.yml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 29d37f0a..959ce565 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -98,7 +98,7 @@ jobs: - name: Build and run tests run: | set -x - cd RIOT + export RIOTBASE=$(pwd)/RIOT # Removing tests/pkg/lvgl due to https://github.com/RIOT-OS/RIOT/issues/20110 # # Skipping examples because ... not sure, did it that way before @@ -108,8 +108,7 @@ jobs: # Skipping peripheral and driver tests because we don't implement those # with wrappers; the valuable ones are those like saul where there are # modules that use Rust. - DIRS=$(make -f makefiles/app_dirs.inc.mk info-applications | grep -v tests/pkg/lvgl |grep -v examples |grep -v periph |grep -v driver) - export RIOTBASE=$(pwd)/RIOT + DIRS=$(make --quiet -C ${RIOTBASE} -f makefiles/app_dirs.inc.mk info-applications | grep -v tests/pkg/lvgl |grep -v examples |grep -v periph |grep -v driver) for D in ${DIRS}; do echo "::group::Building ${D}" cd ${D} From 25d9e34db9b1f42145c3065d5fd4d446e12afa24 Mon Sep 17 00:00:00 2001 From: chrysn Date: Sat, 2 Dec 2023 23:55:59 +0100 Subject: [PATCH 49/50] CI: Run own tests instead of RIOT tests The tests are now run using individual steps for the same reasons the examples were switched over. In [68], the tests were accidentally switched over to running all RIOT instead of all own tests (which may also make sense but is really excessive, and most importantly lost the own tests) [68]: https://github.com/RIOT-OS/rust-riot-wrappers/pull/68 --- .github/workflows/test.yml | 80 ++++++++++++++++++-------------------- 1 file changed, 38 insertions(+), 42 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 959ce565..951647c1 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -10,13 +10,16 @@ on: workflow_dispatch: jobs: - quick-examples: + examples-and-tests: runs-on: ubuntu-latest container: riot/riotbuild # Best would be "continue until there are errors from different examples *and* different boards" continue-on-error: true strategy: matrix: + # This is the subset of `make -f makefiles/app_dirs.inc.mk + # info-applications` that is probably relevant; more is covered when + # riot-wrappers are updated in RIOT. example: [examples/rust-hello-world, examples/rust-gcoap, tests/rust_minimal] board: [native, sltb001a, samr21-xpro, stk3700] steps: @@ -56,12 +59,31 @@ jobs: run: | make all BOARD=${{ matrix.board }} -C RIOT/${{ matrix.example }} - most-tests: + enumerate-wrappers-tests: runs-on: ubuntu-latest + outputs: + list: ${{ steps.enumerate.outputs.tests }} + steps: + - name: Check out riot-wrappers + uses: actions/checkout@v3 + - name: List tests in riot-wrappers + id: enumerate + run: | + echo "tests=[$(ls -d tests/*/ -1 | sed 's/.*/\"&\"/' | tr '\n' ',' | sed 's/,$//')]" >> "${GITHUB_OUTPUT}" + cat "${GITHUB_OUTPUT}" + - name: Set job summary + run: | + # This doubles as a check to see that our JSON is right + echo 'Local tests were enumerated to be `${{ toJSON(fromJSON( steps.enumerate.outputs.tests )) }}`' >> $GITHUB_STEP_SUMMARY + + wrappers-tests: + runs-on: ubuntu-latest + needs: enumerate-wrappers-tests container: riot/riotbuild strategy: matrix: board: [native, sltb001a, samr21-xpro, stk3700] + testdir: ${{ fromJSON(needs.enumerate-wrappers-tests.outputs.list) }} steps: # common steps start here - name: Check out riot-wrappers @@ -95,51 +117,25 @@ jobs: done # common steps end here - - name: Build and run tests + - name: Build and run test run: | set -x export RIOTBASE=$(pwd)/RIOT - # Removing tests/pkg/lvgl due to https://github.com/RIOT-OS/RIOT/issues/20110 - # - # Skipping examples because ... not sure, did it that way before - # https://github.com/RIOT-OS/rust-riot-wrappers/pull/68, and examples - # are built extra - # - # Skipping peripheral and driver tests because we don't implement those - # with wrappers; the valuable ones are those like saul where there are - # modules that use Rust. - DIRS=$(make --quiet -C ${RIOTBASE} -f makefiles/app_dirs.inc.mk info-applications | grep -v tests/pkg/lvgl |grep -v examples |grep -v periph |grep -v driver) - for D in ${DIRS}; do - echo "::group::Building ${D}" - cd ${D} - if BOARDS=${{ matrix.board }} make info-boards-supported | grep -q . - then - # RIOTNOLINK is a workaround for boards with insufficient memory - # showing as supported but still being known not to be buildable. - # Other CI works around by having RIOT_CI_BUILD set RIOTNOLINK if - # the board is known to not have enough memory from - # BOARD_INSUFFICIENT_MEMORY, but that information is not piped out. - # - # Until a better workaround is found, no boards are linked, and if - # a board does exceed its memory due to a Rust change, that will - # only be spotted when the Rust crate is updated in RIOT and a full - # test with the precise criterion is run. - BOARD=${{ matrix.board }} make all RIOTNOLINK=1 + cd ${{ matrix.testdir }} + if BOARDS=${{ matrix.board }} make info-boards-supported | grep -q . + then + BOARD=${{ matrix.board }} make all - if [ "native" = "${{ matrix.board }}" ] && make test/available BOARD=native - then - echo "::group::Testing ${D}" - make all test BOARD=native - echo "::endgroup::" - fi - BOARD=${{ matrix.board }} make clean - else - echo "Board is not supported for this test, skipping." + if [ "native" = "${{ matrix.board }}" ] && make test/available BOARD=native + then + echo + echo "Testing ${D}" + echo + make all test BOARD=native fi - cd - - echo "::endgroup::" - done - echo "::echo ::off" + else + echo "Board is not supported for this test, skipping." + fi rustfmt: runs-on: ubuntu-latest From 9347da1abba9d5effe72eb17c6a85a9d0db9919e Mon Sep 17 00:00:00 2001 From: chrysn Date: Sun, 3 Dec 2023 00:29:13 +0100 Subject: [PATCH 50/50] CI: Add all-done target to collect matrices --- .github/workflows/test.yml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 951647c1..b52f83d0 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -146,3 +146,11 @@ jobs: fetch-depth: 0 - name: Run cargo-fmt run: cargo fmt --check + + all-done: + needs: [rustfmt, wrappers-tests, examples-and-tests] + # It'd suffice to just do "needs", but GitHub actions insist to have steps + runs-on: ubuntu-latest + steps: + - run: echo "All done" +