From 53637d706af0f8150dd2a076bf1a6918f94ff0af Mon Sep 17 00:00:00 2001 From: Nicolas Menard Date: Wed, 8 Jan 2025 14:02:44 -0500 Subject: [PATCH 1/6] iterate over storage --- micro-rdk/src/common/credentials_storage.rs | 336 ++++++++++++++++++-- micro-rdk/src/esp32/nvs_storage.rs | 24 +- 2 files changed, 327 insertions(+), 33 deletions(-) diff --git a/micro-rdk/src/common/credentials_storage.rs b/micro-rdk/src/common/credentials_storage.rs index dda062c0..adba8b74 100644 --- a/micro-rdk/src/common/credentials_storage.rs +++ b/micro-rdk/src/common/credentials_storage.rs @@ -1,18 +1,19 @@ #![allow(dead_code)] +#![allow(clippy::manual_try_fold)] +use crate::common::grpc::GrpcError; +use hyper::{http::uri::InvalidUri, Uri}; use std::str::FromStr; -use std::{convert::Infallible, error::Error, fmt::Debug, rc::Rc, sync::Mutex}; - -use hyper::Uri; +use std::{error::Error, fmt::Debug, rc::Rc, sync::Mutex}; use crate::{common::grpc::ServerError, proto::app::v1::RobotConfig}; +#[cfg(feature = "ota")] +use crate::common::ota::OtaMetadata; use crate::proto::{ app::v1::CertificateResponse, provisioning::v1::{CloudConfig, SetNetworkCredentialsRequest}, }; - -#[cfg(feature = "ota")] -use crate::common::ota::OtaMetadata; +use thiserror::Error; #[derive(Clone, Default, Debug)] pub struct RobotCredentials { @@ -143,6 +144,10 @@ pub trait StorageDiagnostic { fn log_space_diagnostic(&self); } +#[derive(Error, Debug)] +#[error("empty storage collection")] +pub struct EmptyStorageCollectionError; + #[derive(Default)] struct RAMCredentialStorageInner { robot_creds: Option, @@ -158,6 +163,22 @@ struct RAMCredentialStorageInner { #[derive(Default, Clone)] pub struct RAMStorage(Rc>); +#[derive(Error, Debug)] +pub enum RAMStorageError { + #[error(transparent)] + ParseUriError(#[from] InvalidUri), + #[error(transparent)] + NVSCollectionEmpty(#[from] EmptyStorageCollectionError), + #[error("object not found")] + NotFound, +} + +impl From for ServerError { + fn from(value: RAMStorageError) -> Self { + Self::new(GrpcError::RpcUnavailable, Some(value.into())) + } +} + impl RAMStorage { pub fn new() -> Self { Self(Rc::new(Mutex::new(RAMCredentialStorageInner { @@ -174,42 +195,76 @@ impl RAMStorage { #[cfg(feature = "ota")] impl OtaMetadataStorage for RAMStorage { - type Error = Infallible; + type Error = RAMStorageError; fn has_ota_metadata(&self) -> bool { let inner_ref = self.0.lock().unwrap(); inner_ref.ota_metadata.is_some() } fn store_ota_metadata(&self, ota_metadata: OtaMetadata) -> Result<(), Self::Error> { let mut inner_ref = self.0.lock().unwrap(); - let _ = inner_ref.ota_metadata.insert(ota_metadata); + let _ = inner_ref.ota_metadata.insert(ota_metadata.clone()); Ok(()) } fn get_ota_metadata(&self) -> Result { let inner_ref = self.0.lock().unwrap(); - Ok(inner_ref.ota_metadata.clone().unwrap_or_default()) + inner_ref + .ota_metadata + .clone() + .ok_or(RAMStorageError::NotFound) } fn reset_ota_metadata(&self) -> Result<(), Self::Error> { let _ = self.0.lock().unwrap().ota_metadata.take(); Ok(()) } } +#[cfg(feature = "ota")] +impl OtaMetadataStorage for Iterable +where + for<'a> &'a Iterable: IntoIterator, + Storage::Error: From, +{ + type Error = Storage::Error; + fn has_ota_metadata(&self) -> bool { + self.into_iter().any(OtaMetadataStorage::has_ota_metadata) + } + fn get_ota_metadata(&self) -> Result { + self.into_iter().fold( + Err::<_, Self::Error>(EmptyStorageCollectionError.into()), + |val, s| val.or(s.get_ota_metadata()), + ) + } + fn reset_ota_metadata(&self) -> Result<(), Self::Error> { + self.into_iter().fold( + Err::<_, Self::Error>(EmptyStorageCollectionError.into()), + |val, s| val.or(s.reset_ota_metadata()), + ) + } + fn store_ota_metadata(&self, ota_metadata: OtaMetadata) -> Result<(), Self::Error> { + self.into_iter().fold( + Err::<_, Self::Error>(EmptyStorageCollectionError.into()), + |val, s| val.or(s.store_ota_metadata(ota_metadata.clone())), + ) + } +} impl RobotConfigurationStorage for RAMStorage { - type Error = >::Error; + type Error = RAMStorageError; fn has_robot_credentials(&self) -> bool { let inner_ref = self.0.lock().unwrap(); inner_ref.robot_creds.is_some() } fn store_robot_credentials(&self, cfg: CloudConfig) -> Result<(), Self::Error> { - let creds: RobotCredentials = cfg.try_into()?; + let creds: RobotCredentials = cfg.clone().try_into()?; let mut inner_ref = self.0.lock().unwrap(); let _ = inner_ref.robot_creds.insert(creds); Ok(()) } fn get_robot_credentials(&self) -> Result { let inner_ref = self.0.lock().unwrap(); - let cfg = inner_ref.robot_creds.clone().unwrap_or_default().clone(); - Ok(cfg) + inner_ref + .robot_creds + .clone() + .ok_or(RAMStorageError::NotFound) } fn reset_robot_credentials(&self) -> Result<(), Self::Error> { let mut inner_ref = self.0.lock().unwrap(); @@ -225,14 +280,12 @@ impl RobotConfigurationStorage for RAMStorage { Ok(()) } fn get_robot_configuration(&self) -> Result { - Ok(self - .0 + self.0 .lock() .unwrap() .robot_config .clone() - .unwrap_or_default() - .clone()) + .ok_or(RAMStorageError::NotFound) } fn reset_robot_configuration(&self) -> Result<(), Self::Error> { let _ = self.0.lock().unwrap().robot_config.take(); @@ -240,8 +293,7 @@ impl RobotConfigurationStorage for RAMStorage { } fn get_tls_certificate(&self) -> Result { let inner_ref = self.0.lock().unwrap(); - let creds = inner_ref.tls_cert.clone().unwrap_or_default(); - Ok(creds) + inner_ref.tls_cert.clone().ok_or(RAMStorageError::NotFound) } fn has_tls_certificate(&self) -> bool { let inner_ref = self.0.lock().unwrap(); @@ -249,7 +301,7 @@ impl RobotConfigurationStorage for RAMStorage { } fn store_tls_certificate(&self, creds: TlsCertificate) -> Result<(), Self::Error> { let mut inner_ref = self.0.lock().unwrap(); - let _ = inner_ref.tls_cert.insert(creds); + let _ = inner_ref.tls_cert.insert(creds.clone()); Ok(()) } fn reset_tls_certificate(&self) -> Result<(), Self::Error> { @@ -264,11 +316,11 @@ impl RobotConfigurationStorage for RAMStorage { } fn get_app_address(&self) -> Result { let inner_ref = self.0.lock().unwrap(); - inner_ref + Ok(inner_ref .app_address .clone() .unwrap_or_default() - .parse::() + .parse::()?) } fn reset_app_address(&self) -> Result<(), Self::Error> { let mut inner_ref = self.0.lock().unwrap(); @@ -281,12 +333,110 @@ impl RobotConfigurationStorage for RAMStorage { } } +impl RobotConfigurationStorage for Iterable +where + for<'a> &'a Iterable: IntoIterator, + Storage::Error: From, +{ + type Error = Storage::Error; + fn has_robot_credentials(&self) -> bool { + self.into_iter() + .any(RobotConfigurationStorage::has_robot_credentials) + } + fn has_tls_certificate(&self) -> bool { + self.into_iter() + .any(RobotConfigurationStorage::has_tls_certificate) + } + fn has_app_address(&self) -> bool { + self.into_iter() + .any(RobotConfigurationStorage::has_app_address) + } + fn has_robot_configuration(&self) -> bool { + self.into_iter() + .any(RobotConfigurationStorage::has_robot_configuration) + } + fn get_robot_credentials(&self) -> Result { + self.into_iter().fold( + Err::<_, Self::Error>(EmptyStorageCollectionError.into()), + |val, s| val.or(s.get_robot_credentials()), + ) + } + fn get_tls_certificate(&self) -> Result { + self.into_iter().fold( + Err::<_, Self::Error>(EmptyStorageCollectionError.into()), + |val, s| val.or(s.get_tls_certificate()), + ) + } + fn get_app_address(&self) -> Result { + self.into_iter().fold( + Err::<_, Self::Error>(EmptyStorageCollectionError.into()), + |val, s| val.or(s.get_app_address()), + ) + } + fn store_app_address(&self, uri: &str) -> Result<(), Self::Error> { + self.into_iter().fold( + Err::<_, Self::Error>(EmptyStorageCollectionError.into()), + |val, s| val.or(s.store_app_address(uri)), + ) + } + fn store_tls_certificate(&self, creds: TlsCertificate) -> Result<(), Self::Error> { + self.into_iter().fold( + Err::<_, Self::Error>(EmptyStorageCollectionError.into()), + |val, s| val.or(s.store_tls_certificate(creds.clone())), + ) + } + fn store_robot_configuration(&self, cfg: &RobotConfig) -> Result<(), Self::Error> { + self.into_iter().fold( + Err::<_, Self::Error>(EmptyStorageCollectionError.into()), + |val, s| val.or(s.store_robot_configuration(cfg)), + ) + } + fn store_robot_credentials(&self, cfg: CloudConfig) -> Result<(), Self::Error> { + self.into_iter().fold( + Err::<_, Self::Error>(EmptyStorageCollectionError.into()), + |val, s| val.or(s.store_robot_credentials(cfg.clone())), + ) + } + fn get_robot_configuration(&self) -> Result { + self.into_iter().fold( + Err::<_, Self::Error>(EmptyStorageCollectionError.into()), + |val, s| val.or(s.get_robot_configuration()), + ) + } + fn reset_app_address(&self) -> Result<(), Self::Error> { + self.into_iter().fold( + Err::<_, Self::Error>(EmptyStorageCollectionError.into()), + |val, s| val.or(s.reset_app_address()), + ) + } + fn reset_robot_configuration(&self) -> Result<(), Self::Error> { + self.into_iter().fold( + Err::<_, Self::Error>(EmptyStorageCollectionError.into()), + |val, s| val.or(s.reset_robot_configuration()), + ) + } + fn reset_tls_certificate(&self) -> Result<(), Self::Error> { + self.into_iter().fold( + Err::<_, Self::Error>(EmptyStorageCollectionError.into()), + |val, s| val.or(s.reset_tls_certificate()), + ) + } + fn reset_robot_credentials(&self) -> Result<(), Self::Error> { + self.into_iter().fold( + Err::<_, Self::Error>(EmptyStorageCollectionError.into()), + |val, s| val.or(s.reset_robot_credentials()), + ) + } +} + impl WifiCredentialStorage for RAMStorage { - type Error = Infallible; + type Error = RAMStorageError; fn get_wifi_credentials(&self) -> Result { let inner_ref = self.0.lock().unwrap(); - let creds = inner_ref.wifi_creds.clone().unwrap_or_default(); - Ok(creds) + inner_ref + .wifi_creds + .clone() + .ok_or(RAMStorageError::NotFound) } fn has_wifi_credentials(&self) -> bool { let inner_ref = self.0.lock().unwrap(); @@ -294,7 +444,7 @@ impl WifiCredentialStorage for RAMStorage { } fn store_wifi_credentials(&self, creds: WifiCredentials) -> Result<(), Self::Error> { let mut inner_ref = self.0.lock().unwrap(); - let _ = inner_ref.wifi_creds.insert(creds); + let _ = inner_ref.wifi_creds.insert(creds.clone()); Ok(()) } fn reset_wifi_credentials(&self) -> Result<(), Self::Error> { @@ -304,12 +454,140 @@ impl WifiCredentialStorage for RAMStorage { } } +impl WifiCredentialStorage for Iterable +where + for<'a> &'a Iterable: IntoIterator, + Storage::Error: From, +{ + type Error = Storage::Error; + fn has_wifi_credentials(&self) -> bool { + self.into_iter() + .any(WifiCredentialStorage::has_wifi_credentials) + } + fn get_wifi_credentials(&self) -> Result { + self.into_iter().fold( + Err::<_, Self::Error>(EmptyStorageCollectionError.into()), + |val, s| val.or(s.get_wifi_credentials()), + ) + } + fn store_wifi_credentials(&self, creds: WifiCredentials) -> Result<(), Self::Error> { + self.into_iter().fold( + Err::<_, Self::Error>(EmptyStorageCollectionError.into()), + |val, s| val.or(s.store_wifi_credentials(creds.clone())), + ) + } + fn reset_wifi_credentials(&self) -> Result<(), Self::Error> { + self.into_iter().fold( + Err::<_, Self::Error>(EmptyStorageCollectionError.into()), + |val, s| val.or(s.reset_wifi_credentials()), + ) + } +} + impl StorageDiagnostic for RAMStorage { fn log_space_diagnostic(&self) {} } -impl From for ServerError { - fn from(_: Infallible) -> Self { - unreachable!() +impl StorageDiagnostic for Iterable +where + for<'a> &'a Iterable: IntoIterator, +{ + fn log_space_diagnostic(&self) { + self.into_iter() + .for_each(StorageDiagnostic::log_space_diagnostic); + } +} + +#[cfg(test)] +mod tests { + use crate::common::credentials_storage::{ + RAMStorage, RAMStorageError, RobotConfigurationStorage, + }; + use crate::proto::provisioning::v1::CloudConfig; + use std::collections::HashSet; + #[test_log::test] + fn test_vec_storage_empty() { + let v: Vec = vec![]; + + assert!(!v.has_robot_credentials()); + let err = v.get_robot_credentials(); + assert!(err.is_err()); + assert!(matches!( + err.unwrap_err(), + RAMStorageError::NVSCollectionEmpty(_) + )); + } + + #[test_log::test] + fn test_trait_impls() { + // compile time check for trait implementation over collections + fn is_robot_configuration_storage() {} + is_robot_configuration_storage::>(); + is_robot_configuration_storage::<[RAMStorage; 2]>(); + is_robot_configuration_storage::>(); + } + + #[test_log::test] + fn test_vec_storage() { + let ram1 = RAMStorage::new(); + let ram2 = RAMStorage::new(); + let ram3 = RAMStorage::new(); + let v: Vec = vec![ram1.clone(), ram2.clone(), ram3.clone()]; + + assert!(!v.has_robot_credentials()); + assert!(ram2 + .store_robot_credentials(CloudConfig { + app_address: "http://downloadramstorage.org".to_owned(), + id: "ram2".to_owned(), + secret: "secret".to_owned() + }) + .is_ok()); + assert!(v.has_robot_credentials()); + let cred = v.get_robot_credentials(); + assert!(cred.is_ok()); + let cred = cred.unwrap(); + assert_eq!(cred.robot_id, "ram2"); + + assert!(ram1 + .store_robot_credentials(CloudConfig { + app_address: "http://downloadramstorage.org".to_owned(), + id: "ram1".to_owned(), + secret: "secret".to_owned() + }) + .is_ok()); + assert!(ram2 + .store_robot_credentials(CloudConfig { + app_address: "http://downloadramstorage.org".to_owned(), + id: "ram2".to_owned(), + secret: "secret".to_owned() + }) + .is_ok()); + assert!(v.has_robot_credentials()); + let cred = v.get_robot_credentials(); + assert!(cred.is_ok()); + let cred = cred.unwrap(); + // if multiple credentials stored first one should be returned + assert_eq!(cred.robot_id, "ram1"); + + // reset always remove credentials from all storage in array + assert!(v.reset_robot_credentials().is_ok()); + assert!(!v.has_robot_credentials()); + + assert!(v + .store_robot_credentials(CloudConfig { + app_address: "http://downloadramstorage.org".to_owned(), + id: "vec".to_owned(), + secret: "secret".to_owned() + }) + .is_ok()); + assert!(v.has_robot_credentials()); + let cred = v.get_robot_credentials(); + assert!(cred.is_ok()); + let cred = cred.unwrap(); + assert_eq!(cred.robot_id, "vec"); + let cred = ram1.get_robot_credentials(); + assert!(cred.is_ok()); + let cred = cred.unwrap(); + assert_eq!(cred.robot_id, "vec"); } } diff --git a/micro-rdk/src/esp32/nvs_storage.rs b/micro-rdk/src/esp32/nvs_storage.rs index 46f4b612..92b2ad27 100644 --- a/micro-rdk/src/esp32/nvs_storage.rs +++ b/micro-rdk/src/esp32/nvs_storage.rs @@ -8,12 +8,13 @@ use thiserror::Error; use crate::{ common::{ credentials_storage::{ - RobotConfigurationStorage, RobotCredentials, StorageDiagnostic, TlsCertificate, - WifiCredentialStorage, WifiCredentials, + EmptyStorageCollectionError, RobotConfigurationStorage, RobotCredentials, + StorageDiagnostic, TlsCertificate, WifiCredentialStorage, WifiCredentials, }, grpc::{GrpcError, ServerError}, }, esp32::esp_idf_svc::{ + handle::RawHandle, nvs::{EspCustomNvs, EspCustomNvsPartition, EspNvs}, sys::{esp, nvs_get_stats, nvs_stats_t, EspError, ESP_ERR_INVALID_ARG}, }, @@ -34,6 +35,8 @@ pub enum NVSStorageError { NVSValueDecodeError(#[from] DecodeError), #[error(transparent)] NVSUriParseError(#[from] InvalidUri), + #[error("nvs collection empty")] + NVSCollectionEmpty(#[from] EmptyStorageCollectionError), } #[derive(Clone)] @@ -42,19 +45,32 @@ pub struct NVSStorage { // so inner mutability can be achieves safely with RefCell nvs: Rc>, partition_name: CString, + part: EspCustomNvsPartition, +} + +#[derive(Clone)] +pub struct MultiNVSStorage { + parts: Vec, +} + +impl MultiNVSStorage { + pub fn new(parts: Vec) -> Self { + Self { parts } + } } impl NVSStorage { // taking partition name as argument so we can use another NVS part name if we want to. pub fn new(partition_name: &str) -> Result { let partition: EspCustomNvsPartition = EspCustomNvsPartition::take(partition_name)?; - let nvs = EspNvs::new(partition, "VIAM_NS", true)?; + let nvs = EspNvs::new(partition.clone(), "VIAM_NS", true)?; Ok(Self { nvs: Rc::new(nvs.into()), partition_name: CString::new(partition_name).map_err(|_| { EspError::from_non_zero(NonZeroI32::new(ESP_ERR_INVALID_ARG).unwrap()) })?, + part: partition, }) } @@ -136,7 +152,7 @@ impl StorageDiagnostic for NVSStorage { fn log_space_diagnostic(&self) { let mut stats: nvs_stats_t = Default::default(); if let Err(err) = - esp!(unsafe { nvs_get_stats(self.partition_name.as_ptr(), &mut stats as *mut _) }) + esp!(unsafe { nvs_get_stats(self.part.handle() as _, &mut stats as *mut _) }) { log::error!("could not acquire NVS stats: {:?}", err); return; From 312ff5fa4d9e25e67b702e576b46d166ecc9c3f7 Mon Sep 17 00:00:00 2001 From: Nicolas Menard Date: Wed, 8 Jan 2025 15:47:18 -0500 Subject: [PATCH 2/6] pass object to store as reference --- micro-rdk-ffi/src/ffi/runtime.rs | 2 +- micro-rdk-server/esp32/main.rs | 4 +-- micro-rdk-server/native/main.rs | 2 +- micro-rdk/src/common/conn/viam.rs | 10 +++--- micro-rdk/src/common/credentials_storage.rs | 40 ++++++++++----------- micro-rdk/src/common/ota.rs | 2 +- micro-rdk/src/common/provisioning/server.rs | 5 +-- micro-rdk/src/esp32/nvs_storage.rs | 18 ++++++---- templates/project/src/main.rs | 4 +-- 9 files changed, 47 insertions(+), 40 deletions(-) diff --git a/micro-rdk-ffi/src/ffi/runtime.rs b/micro-rdk-ffi/src/ffi/runtime.rs index 260601e9..8f0816c7 100644 --- a/micro-rdk-ffi/src/ffi/runtime.rs +++ b/micro-rdk-ffi/src/ffi/runtime.rs @@ -269,7 +269,7 @@ pub unsafe extern "C" fn viam_server_start(ctx: *mut viam_server_context) -> via None }.expect("has_robot_config set in cfg, but build-time configuration failed to set robot credentials"); ram_storage - .store_robot_credentials(cloud_conf) + .store_robot_credentials(&cloud_conf) .expect("Failed to store cloud config"); if ROBOT_APP_ADDRESS.is_some() { ram_storage diff --git a/micro-rdk-server/esp32/main.rs b/micro-rdk-server/esp32/main.rs index 76d1e870..e0f7eacc 100644 --- a/micro-rdk-server/esp32/main.rs +++ b/micro-rdk-server/esp32/main.rs @@ -82,7 +82,7 @@ mod esp32 { if SSID.is_some() && PASS.is_some() { log::info!("Storing static values from build time wifi configuration to NVS"); storage - .store_wifi_credentials(WifiCredentials::new( + .store_wifi_credentials(&WifiCredentials::new( SSID.unwrap().to_string(), PASS.unwrap().to_string(), )) @@ -96,7 +96,7 @@ mod esp32 { log::info!("Storing static values from build time robot configuration to NVS"); storage .store_robot_credentials( - RobotCredentials::new( + &RobotCredentials::new( ROBOT_ID.unwrap().to_string(), ROBOT_SECRET.unwrap().to_string(), ) diff --git a/micro-rdk-server/native/main.rs b/micro-rdk-server/native/main.rs index f52ef9c0..b2aaa447 100755 --- a/micro-rdk-server/native/main.rs +++ b/micro-rdk-server/native/main.rs @@ -47,7 +47,7 @@ mod native { log::info!("Storing static values from build time robot configuration"); storage .store_robot_credentials( - RobotCredentials::new( + &RobotCredentials::new( ROBOT_ID.unwrap().to_string(), ROBOT_SECRET.unwrap().to_string(), ) diff --git a/micro-rdk/src/common/conn/viam.rs b/micro-rdk/src/common/conn/viam.rs index 07e09d51..7c4621f0 100644 --- a/micro-rdk/src/common/conn/viam.rs +++ b/micro-rdk/src/common/conn/viam.rs @@ -609,7 +609,7 @@ where .await .map(|cert_resp| { let cert: TlsCertificate = cert_resp.into(); - match self.storage.store_tls_certificate(cert.clone()) { + match self.storage.store_tls_certificate(&cert) { Ok(_) => { log::debug!("stored TLS certificate received by app"); } @@ -1290,7 +1290,7 @@ mod tests { secret: "".to_string(), app_address: LOCALHOST_URI.to_owned(), }; - assert!(ram_storage.store_robot_credentials(creds).is_ok()); + assert!(ram_storage.store_robot_credentials(&creds).is_ok()); let mdns = NativeMdns::new("".to_owned(), network.get_ip()); assert!(mdns.is_ok()); @@ -1350,7 +1350,7 @@ mod tests { secret: "".to_string(), app_address: LOCALHOST_URI.to_owned(), }; - assert!(ram_storage.store_robot_credentials(creds).is_ok()); + assert!(ram_storage.store_robot_credentials(&creds).is_ok()); let mdns = NativeMdns::new("".to_owned(), network.get_ip()); assert!(mdns.is_ok()); @@ -1460,7 +1460,7 @@ mod tests { app_address: LOCALHOST_URI.to_owned(), }; - assert!(ram_storage.store_robot_credentials(creds).is_ok()); + assert!(ram_storage.store_robot_credentials(&creds).is_ok()); let mdns = NativeMdns::new("".to_owned(), network.get_ip()); assert!(mdns.is_ok()); @@ -1787,7 +1787,7 @@ mod tests { _ => panic!("oops expected ipv4"), }; - ram_storage.store_robot_credentials(creds).unwrap(); + ram_storage.store_robot_credentials(&creds).unwrap(); let mut a = ViamServerBuilder::new(ram_storage); let mdns = NativeMdns::new("".to_owned(), Ipv4Addr::new(0, 0, 0, 0)).unwrap(); diff --git a/micro-rdk/src/common/credentials_storage.rs b/micro-rdk/src/common/credentials_storage.rs index adba8b74..c0352fd8 100644 --- a/micro-rdk/src/common/credentials_storage.rs +++ b/micro-rdk/src/common/credentials_storage.rs @@ -103,7 +103,7 @@ impl From for TlsCertificate { pub trait WifiCredentialStorage { type Error: Error + Debug + Into; fn has_wifi_credentials(&self) -> bool; - fn store_wifi_credentials(&self, creds: WifiCredentials) -> Result<(), Self::Error>; + fn store_wifi_credentials(&self, creds: &WifiCredentials) -> Result<(), Self::Error>; fn get_wifi_credentials(&self) -> Result; fn reset_wifi_credentials(&self) -> Result<(), Self::Error>; } @@ -111,7 +111,7 @@ pub trait WifiCredentialStorage { pub trait RobotConfigurationStorage { type Error: Error + Debug + Into; fn has_robot_credentials(&self) -> bool; - fn store_robot_credentials(&self, cfg: CloudConfig) -> Result<(), Self::Error>; + fn store_robot_credentials(&self, cfg: &CloudConfig) -> Result<(), Self::Error>; fn get_robot_credentials(&self) -> Result; fn reset_robot_credentials(&self) -> Result<(), Self::Error>; @@ -126,7 +126,7 @@ pub trait RobotConfigurationStorage { fn reset_robot_configuration(&self) -> Result<(), Self::Error>; fn has_tls_certificate(&self) -> bool; - fn store_tls_certificate(&self, creds: TlsCertificate) -> Result<(), Self::Error>; + fn store_tls_certificate(&self, creds: &TlsCertificate) -> Result<(), Self::Error>; fn get_tls_certificate(&self) -> Result; fn reset_tls_certificate(&self) -> Result<(), Self::Error>; } @@ -136,7 +136,7 @@ pub trait OtaMetadataStorage { type Error: Error + Debug + Into; fn has_ota_metadata(&self) -> bool; fn get_ota_metadata(&self) -> Result; - fn store_ota_metadata(&self, ota_metadata: OtaMetadata) -> Result<(), Self::Error>; + fn store_ota_metadata(&self, ota_metadata: &OtaMetadata) -> Result<(), Self::Error>; fn reset_ota_metadata(&self) -> Result<(), Self::Error>; } @@ -200,7 +200,7 @@ impl OtaMetadataStorage for RAMStorage { let inner_ref = self.0.lock().unwrap(); inner_ref.ota_metadata.is_some() } - fn store_ota_metadata(&self, ota_metadata: OtaMetadata) -> Result<(), Self::Error> { + fn store_ota_metadata(&self, ota_metadata: &OtaMetadata) -> Result<(), Self::Error> { let mut inner_ref = self.0.lock().unwrap(); let _ = inner_ref.ota_metadata.insert(ota_metadata.clone()); Ok(()) @@ -239,10 +239,10 @@ where |val, s| val.or(s.reset_ota_metadata()), ) } - fn store_ota_metadata(&self, ota_metadata: OtaMetadata) -> Result<(), Self::Error> { + fn store_ota_metadata(&self, ota_metadata: &OtaMetadata) -> Result<(), Self::Error> { self.into_iter().fold( Err::<_, Self::Error>(EmptyStorageCollectionError.into()), - |val, s| val.or(s.store_ota_metadata(ota_metadata.clone())), + |val, s| val.or(s.store_ota_metadata(ota_metadata)), ) } } @@ -253,7 +253,7 @@ impl RobotConfigurationStorage for RAMStorage { let inner_ref = self.0.lock().unwrap(); inner_ref.robot_creds.is_some() } - fn store_robot_credentials(&self, cfg: CloudConfig) -> Result<(), Self::Error> { + fn store_robot_credentials(&self, cfg: &CloudConfig) -> Result<(), Self::Error> { let creds: RobotCredentials = cfg.clone().try_into()?; let mut inner_ref = self.0.lock().unwrap(); let _ = inner_ref.robot_creds.insert(creds); @@ -299,7 +299,7 @@ impl RobotConfigurationStorage for RAMStorage { let inner_ref = self.0.lock().unwrap(); inner_ref.tls_cert.is_some() } - fn store_tls_certificate(&self, creds: TlsCertificate) -> Result<(), Self::Error> { + fn store_tls_certificate(&self, creds: &TlsCertificate) -> Result<(), Self::Error> { let mut inner_ref = self.0.lock().unwrap(); let _ = inner_ref.tls_cert.insert(creds.clone()); Ok(()) @@ -379,10 +379,10 @@ where |val, s| val.or(s.store_app_address(uri)), ) } - fn store_tls_certificate(&self, creds: TlsCertificate) -> Result<(), Self::Error> { + fn store_tls_certificate(&self, creds: &TlsCertificate) -> Result<(), Self::Error> { self.into_iter().fold( Err::<_, Self::Error>(EmptyStorageCollectionError.into()), - |val, s| val.or(s.store_tls_certificate(creds.clone())), + |val, s| val.or(s.store_tls_certificate(creds)), ) } fn store_robot_configuration(&self, cfg: &RobotConfig) -> Result<(), Self::Error> { @@ -391,10 +391,10 @@ where |val, s| val.or(s.store_robot_configuration(cfg)), ) } - fn store_robot_credentials(&self, cfg: CloudConfig) -> Result<(), Self::Error> { + fn store_robot_credentials(&self, cfg: &CloudConfig) -> Result<(), Self::Error> { self.into_iter().fold( Err::<_, Self::Error>(EmptyStorageCollectionError.into()), - |val, s| val.or(s.store_robot_credentials(cfg.clone())), + |val, s| val.or(s.store_robot_credentials(cfg)), ) } fn get_robot_configuration(&self) -> Result { @@ -442,7 +442,7 @@ impl WifiCredentialStorage for RAMStorage { let inner_ref = self.0.lock().unwrap(); inner_ref.wifi_creds.is_some() } - fn store_wifi_credentials(&self, creds: WifiCredentials) -> Result<(), Self::Error> { + fn store_wifi_credentials(&self, creds: &WifiCredentials) -> Result<(), Self::Error> { let mut inner_ref = self.0.lock().unwrap(); let _ = inner_ref.wifi_creds.insert(creds.clone()); Ok(()) @@ -470,10 +470,10 @@ where |val, s| val.or(s.get_wifi_credentials()), ) } - fn store_wifi_credentials(&self, creds: WifiCredentials) -> Result<(), Self::Error> { + fn store_wifi_credentials(&self, creds: &WifiCredentials) -> Result<(), Self::Error> { self.into_iter().fold( Err::<_, Self::Error>(EmptyStorageCollectionError.into()), - |val, s| val.or(s.store_wifi_credentials(creds.clone())), + |val, s| val.or(s.store_wifi_credentials(creds)), ) } fn reset_wifi_credentials(&self) -> Result<(), Self::Error> { @@ -536,7 +536,7 @@ mod tests { assert!(!v.has_robot_credentials()); assert!(ram2 - .store_robot_credentials(CloudConfig { + .store_robot_credentials(&CloudConfig { app_address: "http://downloadramstorage.org".to_owned(), id: "ram2".to_owned(), secret: "secret".to_owned() @@ -549,14 +549,14 @@ mod tests { assert_eq!(cred.robot_id, "ram2"); assert!(ram1 - .store_robot_credentials(CloudConfig { + .store_robot_credentials(&CloudConfig { app_address: "http://downloadramstorage.org".to_owned(), id: "ram1".to_owned(), secret: "secret".to_owned() }) .is_ok()); assert!(ram2 - .store_robot_credentials(CloudConfig { + .store_robot_credentials(&CloudConfig { app_address: "http://downloadramstorage.org".to_owned(), id: "ram2".to_owned(), secret: "secret".to_owned() @@ -574,7 +574,7 @@ mod tests { assert!(!v.has_robot_credentials()); assert!(v - .store_robot_credentials(CloudConfig { + .store_robot_credentials(&CloudConfig { app_address: "http://downloadramstorage.org".to_owned(), id: "vec".to_owned(), secret: "secret".to_owned() diff --git a/micro-rdk/src/common/ota.rs b/micro-rdk/src/common/ota.rs index 3f8604a7..b3321af9 100644 --- a/micro-rdk/src/common/ota.rs +++ b/micro-rdk/src/common/ota.rs @@ -549,7 +549,7 @@ impl OtaService { log::info!("updating firmware metadata in NVS"); self.storage - .store_ota_metadata(OtaMetadata { + .store_ota_metadata(&OtaMetadata { version: self.pending_version.clone(), }) .map_err(|e| OtaError::Other(e.to_string()))?; diff --git a/micro-rdk/src/common/provisioning/server.rs b/micro-rdk/src/common/provisioning/server.rs index 106c8f7b..a22ec612 100644 --- a/micro-rdk/src/common/provisioning/server.rs +++ b/micro-rdk/src/common/provisioning/server.rs @@ -256,7 +256,7 @@ where })?; self.storage - .store_wifi_credentials(creds) + .store_wifi_credentials(&creds) .map_err(|e| ServerError::new(GrpcError::RpcInternal, Some(Box::new(e.into()))))?; let resp = SetNetworkCredentialsResponse::default(); @@ -331,7 +331,8 @@ where fn set_smart_machine_credentials(&self, body: Bytes) -> Result { let creds = SetSmartMachineCredentialsRequest::decode(body).map_err(|_| GrpcError::RpcInternal)?; - self.storage.store_robot_credentials(creds.cloud.unwrap())?; + self.storage + .store_robot_credentials(creds.cloud.as_ref().unwrap())?; let resp = SetSmartMachineCredentialsResponse::default(); let len = resp.encoded_len(); diff --git a/micro-rdk/src/esp32/nvs_storage.rs b/micro-rdk/src/esp32/nvs_storage.rs index 92b2ad27..cc3ded6d 100644 --- a/micro-rdk/src/esp32/nvs_storage.rs +++ b/micro-rdk/src/esp32/nvs_storage.rs @@ -205,7 +205,7 @@ impl OtaMetadataStorage for NVSStorage { let version = self.get_string(NVS_OTA_VERSION_KEY)?; Ok(OtaMetadata { version }) } - fn store_ota_metadata(&self, ota_metadata: OtaMetadata) -> Result<(), Self::Error> { + fn store_ota_metadata(&self, ota_metadata: &OtaMetadata) -> Result<(), Self::Error> { self.set_string(NVS_OTA_VERSION_KEY, &ota_metadata.version) } fn reset_ota_metadata(&self) -> Result<(), Self::Error> { @@ -244,7 +244,7 @@ impl RobotConfigurationStorage for NVSStorage { self.erase_key(NVS_ROBOT_APP_ADDRESS) } - fn store_robot_credentials(&self, cfg: CloudConfig) -> Result<(), Self::Error> { + fn store_robot_credentials(&self, cfg: &CloudConfig) -> Result<(), Self::Error> { self.set_string(NVS_ROBOT_SECRET_KEY, &cfg.secret)?; self.set_string(NVS_ROBOT_ID_KEY, &cfg.id)?; self.set_string(NVS_ROBOT_APP_ADDRESS, &cfg.app_address)?; @@ -290,9 +290,15 @@ impl RobotConfigurationStorage for NVSStorage { }) } - fn store_tls_certificate(&self, creds: TlsCertificate) -> Result<(), Self::Error> { - self.set_blob(NVS_TLS_CERTIFICATE_KEY, Bytes::from(creds.certificate))?; - self.set_blob(NVS_TLS_PRIVATE_KEY_KEY, Bytes::from(creds.private_key))?; + fn store_tls_certificate(&self, creds: &TlsCertificate) -> Result<(), Self::Error> { + self.set_blob( + NVS_TLS_CERTIFICATE_KEY, + Bytes::from(creds.certificate.clone()), + )?; + self.set_blob( + NVS_TLS_PRIVATE_KEY_KEY, + Bytes::from(creds.private_key.clone()), + )?; Ok(()) } @@ -316,7 +322,7 @@ impl WifiCredentialStorage for NVSStorage { Ok(WifiCredentials { ssid, pwd }) } - fn store_wifi_credentials(&self, creds: WifiCredentials) -> Result<(), Self::Error> { + fn store_wifi_credentials(&self, creds: &WifiCredentials) -> Result<(), Self::Error> { self.set_string(NVS_WIFI_SSID_KEY, &creds.ssid)?; self.set_string(NVS_WIFI_PASSWORD_KEY, &creds.pwd)?; Ok(()) diff --git a/templates/project/src/main.rs b/templates/project/src/main.rs index 84e2fec5..7b7b725e 100644 --- a/templates/project/src/main.rs +++ b/templates/project/src/main.rs @@ -72,7 +72,7 @@ fn main() { if SSID.is_some() && PASS.is_some() { log::info!("Storing static values from build time wifi configuration to NVS"); storage - .store_wifi_credentials(WifiCredentials::new( + .store_wifi_credentials(&WifiCredentials::new( SSID.unwrap().to_string(), PASS.unwrap().to_string(), )) @@ -87,7 +87,7 @@ fn main() { log::info!("Storing static values from build time robot configuration to NVS"); storage .store_robot_credentials( - RobotCredentials::new( + &RobotCredentials::new( ROBOT_ID.unwrap().to_string(), ROBOT_SECRET.unwrap().to_string(), ) From 75d8b3272a540e3b0b861b04cff9c9ddbf32d82a Mon Sep 17 00:00:00 2001 From: Nicolas Menard Date: Wed, 8 Jan 2025 15:53:05 -0500 Subject: [PATCH 3/6] remove partition_name --- micro-rdk/src/esp32/nvs_storage.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/micro-rdk/src/esp32/nvs_storage.rs b/micro-rdk/src/esp32/nvs_storage.rs index cc3ded6d..3a1749d1 100644 --- a/micro-rdk/src/esp32/nvs_storage.rs +++ b/micro-rdk/src/esp32/nvs_storage.rs @@ -2,7 +2,7 @@ use bytes::Bytes; use hyper::{http::uri::InvalidUri, Uri}; use prost::{DecodeError, Message}; -use std::{cell::RefCell, ffi::CString, num::NonZeroI32, rc::Rc}; +use std::{cell::RefCell, rc::Rc}; use thiserror::Error; use crate::{ @@ -16,7 +16,7 @@ use crate::{ esp32::esp_idf_svc::{ handle::RawHandle, nvs::{EspCustomNvs, EspCustomNvsPartition, EspNvs}, - sys::{esp, nvs_get_stats, nvs_stats_t, EspError, ESP_ERR_INVALID_ARG}, + sys::{esp, nvs_get_stats, nvs_stats_t, EspError}, }, proto::{app::v1::RobotConfig, provisioning::v1::CloudConfig}, }; @@ -44,7 +44,6 @@ pub struct NVSStorage { // esp-idf-svc partition driver ensures that only one handle of a type can be created // so inner mutability can be achieves safely with RefCell nvs: Rc>, - partition_name: CString, part: EspCustomNvsPartition, } @@ -67,9 +66,6 @@ impl NVSStorage { Ok(Self { nvs: Rc::new(nvs.into()), - partition_name: CString::new(partition_name).map_err(|_| { - EspError::from_non_zero(NonZeroI32::new(ESP_ERR_INVALID_ARG).unwrap()) - })?, part: partition, }) } From fd4d2edf5778ba9362ad672c8e6d0a76be7aae5e Mon Sep 17 00:00:00 2001 From: Nicolas Menard Date: Wed, 8 Jan 2025 15:55:06 -0500 Subject: [PATCH 4/6] remove dead code --- micro-rdk/src/esp32/nvs_storage.rs | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/micro-rdk/src/esp32/nvs_storage.rs b/micro-rdk/src/esp32/nvs_storage.rs index 3a1749d1..a846445f 100644 --- a/micro-rdk/src/esp32/nvs_storage.rs +++ b/micro-rdk/src/esp32/nvs_storage.rs @@ -47,17 +47,6 @@ pub struct NVSStorage { part: EspCustomNvsPartition, } -#[derive(Clone)] -pub struct MultiNVSStorage { - parts: Vec, -} - -impl MultiNVSStorage { - pub fn new(parts: Vec) -> Self { - Self { parts } - } -} - impl NVSStorage { // taking partition name as argument so we can use another NVS part name if we want to. pub fn new(partition_name: &str) -> Result { From 7c17538bce0c7278696193f53921897243df9278 Mon Sep 17 00:00:00 2001 From: Nicolas Menard Date: Wed, 8 Jan 2025 16:23:35 -0500 Subject: [PATCH 5/6] or --- micro-rdk/src/common/credentials_storage.rs | 26 +++++++++++---------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/micro-rdk/src/common/credentials_storage.rs b/micro-rdk/src/common/credentials_storage.rs index c0352fd8..c378cac1 100644 --- a/micro-rdk/src/common/credentials_storage.rs +++ b/micro-rdk/src/common/credentials_storage.rs @@ -230,7 +230,7 @@ where fn get_ota_metadata(&self) -> Result { self.into_iter().fold( Err::<_, Self::Error>(EmptyStorageCollectionError.into()), - |val, s| val.or(s.get_ota_metadata()), + |val, s| val.or_else(|_| s.get_ota_metadata()), ) } fn reset_ota_metadata(&self) -> Result<(), Self::Error> { @@ -242,7 +242,7 @@ where fn store_ota_metadata(&self, ota_metadata: &OtaMetadata) -> Result<(), Self::Error> { self.into_iter().fold( Err::<_, Self::Error>(EmptyStorageCollectionError.into()), - |val, s| val.or(s.store_ota_metadata(ota_metadata)), + |val, s| val.or_else(|_| s.store_ota_metadata(ota_metadata)), ) } } @@ -261,6 +261,7 @@ impl RobotConfigurationStorage for RAMStorage { } fn get_robot_credentials(&self) -> Result { let inner_ref = self.0.lock().unwrap(); + log::info!("get robot creds"); inner_ref .robot_creds .clone() @@ -269,6 +270,7 @@ impl RobotConfigurationStorage for RAMStorage { fn reset_robot_credentials(&self) -> Result<(), Self::Error> { let mut inner_ref = self.0.lock().unwrap(); let _ = inner_ref.robot_creds.take(); + log::info!("reset robot creds"); Ok(()) } @@ -358,49 +360,49 @@ where fn get_robot_credentials(&self) -> Result { self.into_iter().fold( Err::<_, Self::Error>(EmptyStorageCollectionError.into()), - |val, s| val.or(s.get_robot_credentials()), + |val, s| val.or_else(|_| s.get_robot_credentials()), ) } fn get_tls_certificate(&self) -> Result { self.into_iter().fold( Err::<_, Self::Error>(EmptyStorageCollectionError.into()), - |val, s| val.or(s.get_tls_certificate()), + |val, s| val.or_else(|_| s.get_tls_certificate()), ) } fn get_app_address(&self) -> Result { self.into_iter().fold( Err::<_, Self::Error>(EmptyStorageCollectionError.into()), - |val, s| val.or(s.get_app_address()), + |val, s| val.or_else(|_| s.get_app_address()), ) } fn store_app_address(&self, uri: &str) -> Result<(), Self::Error> { self.into_iter().fold( Err::<_, Self::Error>(EmptyStorageCollectionError.into()), - |val, s| val.or(s.store_app_address(uri)), + |val, s| val.or_else(|_| s.store_app_address(uri)), ) } fn store_tls_certificate(&self, creds: &TlsCertificate) -> Result<(), Self::Error> { self.into_iter().fold( Err::<_, Self::Error>(EmptyStorageCollectionError.into()), - |val, s| val.or(s.store_tls_certificate(creds)), + |val, s| val.or_else(|_| s.store_tls_certificate(creds)), ) } fn store_robot_configuration(&self, cfg: &RobotConfig) -> Result<(), Self::Error> { self.into_iter().fold( Err::<_, Self::Error>(EmptyStorageCollectionError.into()), - |val, s| val.or(s.store_robot_configuration(cfg)), + |val, s| val.or_else(|_| s.store_robot_configuration(cfg)), ) } fn store_robot_credentials(&self, cfg: &CloudConfig) -> Result<(), Self::Error> { self.into_iter().fold( Err::<_, Self::Error>(EmptyStorageCollectionError.into()), - |val, s| val.or(s.store_robot_credentials(cfg)), + |val, s| val.or_else(|_| s.store_robot_credentials(cfg)), ) } fn get_robot_configuration(&self) -> Result { self.into_iter().fold( Err::<_, Self::Error>(EmptyStorageCollectionError.into()), - |val, s| val.or(s.get_robot_configuration()), + |val, s| val.or_else(|_| s.get_robot_configuration()), ) } fn reset_app_address(&self) -> Result<(), Self::Error> { @@ -467,13 +469,13 @@ where fn get_wifi_credentials(&self) -> Result { self.into_iter().fold( Err::<_, Self::Error>(EmptyStorageCollectionError.into()), - |val, s| val.or(s.get_wifi_credentials()), + |val, s| val.or_else(|_| s.get_wifi_credentials()), ) } fn store_wifi_credentials(&self, creds: &WifiCredentials) -> Result<(), Self::Error> { self.into_iter().fold( Err::<_, Self::Error>(EmptyStorageCollectionError.into()), - |val, s| val.or(s.store_wifi_credentials(creds)), + |val, s| val.or_else(|_| s.store_wifi_credentials(creds)), ) } fn reset_wifi_credentials(&self) -> Result<(), Self::Error> { From 9fc0988b2749064eaa60aa277f3ca76e0a2ceb32 Mon Sep 17 00:00:00 2001 From: Nicolas Menard Date: Thu, 9 Jan 2025 09:59:39 -0500 Subject: [PATCH 6/6] remove stray comments --- micro-rdk/src/common/credentials_storage.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/micro-rdk/src/common/credentials_storage.rs b/micro-rdk/src/common/credentials_storage.rs index c378cac1..0193ba50 100644 --- a/micro-rdk/src/common/credentials_storage.rs +++ b/micro-rdk/src/common/credentials_storage.rs @@ -261,7 +261,6 @@ impl RobotConfigurationStorage for RAMStorage { } fn get_robot_credentials(&self) -> Result { let inner_ref = self.0.lock().unwrap(); - log::info!("get robot creds"); inner_ref .robot_creds .clone() @@ -270,7 +269,6 @@ impl RobotConfigurationStorage for RAMStorage { fn reset_robot_credentials(&self) -> Result<(), Self::Error> { let mut inner_ref = self.0.lock().unwrap(); let _ = inner_ref.robot_creds.take(); - log::info!("reset robot creds"); Ok(()) }