Skip to content

Commit

Permalink
Fix boot loop when upgrading a device to 0.3.1 (#343)
Browse files Browse the repository at this point in the history
  • Loading branch information
npmenard authored Oct 28, 2024
1 parent 0ee4487 commit 0aca25f
Show file tree
Hide file tree
Showing 7 changed files with 90 additions and 36 deletions.
16 changes: 11 additions & 5 deletions micro-rdk-ffi/src/ffi/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,18 +267,24 @@ pub unsafe extern "C" fn viam_server_start(ctx: *mut viam_server_context) -> via
use micro_rdk::common::credentials_storage::RobotConfigurationStorage;
use micro_rdk::proto::provisioning::v1::CloudConfig;

let mut ram_storage = RAMStorage::new();
// TODO: [RSDK-8923] Get app_address from machine credentials
let cloud_conf = if ROBOT_ID.is_some() && ROBOT_SECRET.is_some() && ROBOT_APP_ADDRESS.is_some() {
let ram_storage = RAMStorage::new();
let cloud_conf = if ROBOT_ID.is_some() && ROBOT_SECRET.is_some() {
Some(CloudConfig {
id: ROBOT_ID.unwrap().to_string(),
secret: ROBOT_SECRET.unwrap().to_string(),
app_address: ROBOT_APP_ADDRESS.unwrap().to_string(),
app_address: "".to_string(),
})
} else {
None
}.expect("has_robot_config set in cfg, but build-time configuration failed to set robot credentials");
ram_storage.store_robot_credentials(cloud_conf);
ram_storage
.store_robot_credentials(cloud_conf)
.expect("Failed to store cloud config");
if ROBOT_APP_ADDRESS.is_some() {
ram_storage
.store_app_address(ROBOT_APP_ADDRESS.unwrap())
.expect("Failed to store app address")
}
ram_storage
};

Expand Down
5 changes: 3 additions & 2 deletions micro-rdk-server/esp32/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,12 +99,13 @@ mod esp32 {
RobotCredentials::new(
ROBOT_ID.unwrap().to_string(),
ROBOT_SECRET.unwrap().to_string(),
ROBOT_APP_ADDRESS.unwrap().to_string(),
)
.expect("Failed to parse app address")
.into(),
)
.expect("Failed to store robot credentials to NVS");
storage
.store_app_address(ROBOT_APP_ADDRESS.unwrap())
.expect("Failed to store app address to NVS")
}
}

Expand Down
5 changes: 3 additions & 2 deletions micro-rdk-server/native/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,13 @@ mod native {
RobotCredentials::new(
ROBOT_ID.unwrap().to_string(),
ROBOT_SECRET.unwrap().to_string(),
ROBOT_APP_ADDRESS.unwrap().to_string(),
)
.expect("Failed to parse app address")
.into(),
)
.expect("Failed to store robot credentials");
storage
.store_app_address(ROBOT_APP_ADDRESS.unwrap())
.expect("Failed to store app address")
}
}

Expand Down
27 changes: 21 additions & 6 deletions micro-rdk/src/common/conn/viam.rs
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,14 @@ where
);

let robot_creds = self.storage.get_robot_credentials().unwrap();
let app_address = self
.storage
.get_app_address()
.or_else(|_| {
let _ = self.storage.store_app_address("https://app.viam.com:443");
self.storage.get_app_address()
})
.unwrap();

// attempt to instantiate an app client
// if we have an unauthenticated or permission denied error, we erase the creds
Expand All @@ -455,11 +463,7 @@ where
#[cfg(not(test))]
panic!("erased credentials restart robot"); // TODO bubble up error and go back in provisioning
}
log::error!(
"couldn't connect to {} reason {:?}",
robot_creds.app_address(),
error
);
log::error!("couldn't connect to {} reason {:?}", app_address, error);
})
.ok();

Expand Down Expand Up @@ -629,7 +633,10 @@ where

async fn connect_to_app(&self) -> Result<AppClient, AppClientError> {
let robot_creds = self.storage.get_robot_credentials().unwrap();
let app_uri = robot_creds.app_address();
let app_uri = self
.storage
.get_app_address()
.unwrap_or("https://app.viam.com:443".parse::<Uri>().unwrap());
let app_client_io = self
.http2_connector
.connect_to(&app_uri)
Expand Down Expand Up @@ -1223,6 +1230,7 @@ mod tests {
fn test_app_permission_denied() {
let _unused = global_network_test_lock();
let ram_storage = RAMStorage::new();
let _ = ram_storage.store_app_address(LOCALHOST_URI);
let network = match local_ip_address::local_ip().expect("error parsing local IP") {
std::net::IpAddr::V4(ip) => ExternallyManagedNetwork::new(ip),
_ => panic!("oops expected ipv4"),
Expand Down Expand Up @@ -1281,6 +1289,8 @@ mod tests {
fn test_app_client_transient_failure() {
let _unused = global_network_test_lock();
let ram_storage = RAMStorage::new();
let _ = ram_storage.store_app_address(LOCALHOST_URI);

let network = match local_ip_address::local_ip().expect("error parsing local IP") {
std::net::IpAddr::V4(ip) => ExternallyManagedNetwork::new(ip),
_ => panic!("oops expected ipv4"),
Expand Down Expand Up @@ -1388,6 +1398,8 @@ mod tests {
fn test_multiple_connection_http2() {
let _unused = global_network_test_lock();
let ram_storage = RAMStorage::new();
let _ = ram_storage.store_app_address(LOCALHOST_URI);

let network = match local_ip_address::local_ip().expect("error parsing local IP") {
std::net::IpAddr::V4(ip) => ExternallyManagedNetwork::new(ip),
_ => panic!("oops expected ipv4"),
Expand Down Expand Up @@ -1539,6 +1551,8 @@ mod tests {
fn test_provisioning() {
let _unused = global_network_test_lock();
let ram_storage = RAMStorage::new();
let _ = ram_storage.store_app_address(LOCALHOST_URI);

let network = match local_ip_address::local_ip().expect("error parsing local IP") {
std::net::IpAddr::V4(ip) => ExternallyManagedNetwork::new(ip),
_ => panic!("oops expected ipv4"),
Expand Down Expand Up @@ -1717,6 +1731,7 @@ mod tests {
secret: "".to_string(),
app_address: LOCALHOST_URI.to_owned(),
};
let _ = ram_storage.store_app_address(LOCALHOST_URI);

let network = match local_ip_address::local_ip().expect("error parsing local IP") {
std::net::IpAddr::V4(ip) => ExternallyManagedNetwork::new(ip),
Expand Down
48 changes: 33 additions & 15 deletions micro-rdk/src/common/credentials_storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,14 @@ use crate::proto::{
pub struct RobotCredentials {
pub(crate) robot_id: String,
pub(crate) robot_secret: String,
pub(crate) app_address: Uri,
}

impl RobotCredentials {
pub fn new(
robot_id: String,
robot_secret: String,
app_address: String,
) -> Result<Self, <Uri as FromStr>::Err> {
Ok(Self {
pub fn new(robot_id: String, robot_secret: String) -> Self {
Self {
robot_secret,
robot_id,
app_address: app_address.parse::<Uri>()?,
})
}
}

pub(crate) fn robot_id(&self) -> &str {
Expand All @@ -38,10 +32,6 @@ impl RobotCredentials {
pub(crate) fn robot_secret(&self) -> &str {
&self.robot_secret
}

pub(crate) fn app_address(&self) -> Uri {
self.app_address.clone()
}
}

impl From<SetNetworkCredentialsRequest> for WifiCredentials {
Expand All @@ -59,15 +49,14 @@ impl TryFrom<CloudConfig> for RobotCredentials {
Ok(Self {
robot_id: value.id,
robot_secret: value.secret,
app_address: value.app_address.parse::<Uri>()?,
})
}
}

impl From<RobotCredentials> for CloudConfig {
fn from(value: RobotCredentials) -> Self {
Self {
app_address: value.app_address.to_string(),
app_address: "".to_string(),
id: value.robot_id,
secret: value.robot_secret,
}
Expand Down Expand Up @@ -122,6 +111,11 @@ pub trait RobotConfigurationStorage {
fn get_robot_credentials(&self) -> Result<RobotCredentials, Self::Error>;
fn reset_robot_credentials(&self) -> Result<(), Self::Error>;

fn has_app_address(&self) -> bool;
fn store_app_address(&self, uri: &str) -> Result<(), Self::Error>;
fn get_app_address(&self) -> Result<Uri, Self::Error>;
fn reset_app_address(&self) -> Result<(), Self::Error>;

fn has_robot_configuration(&self) -> bool;
fn store_robot_configuration(&self, cfg: &RobotConfig) -> Result<(), Self::Error>;
fn get_robot_configuration(&self) -> Result<RobotConfig, Self::Error>;
Expand All @@ -139,6 +133,7 @@ struct RAMCredentialStorageInner {
robot_config: Option<RobotConfig>,
wifi_creds: Option<WifiCredentials>,
tls_cert: Option<TlsCertificate>,
app_address: Option<String>,
}

/// Simple CrendentialStorage made for testing purposes
Expand All @@ -152,6 +147,7 @@ impl RAMStorage {
robot_config: None,
wifi_creds: None,
tls_cert: None,
app_address: None,
})))
}
}
Expand Down Expand Up @@ -219,6 +215,28 @@ impl RobotConfigurationStorage for RAMStorage {
let _ = inner_ref.tls_cert.take();
Ok(())
}
fn store_app_address(&self, uri: &str) -> Result<(), Self::Error> {
let mut inner_ref = self.0.lock().unwrap();
let _ = inner_ref.app_address.insert(uri.to_string());
Ok(())
}
fn get_app_address(&self) -> Result<Uri, Self::Error> {
let inner_ref = self.0.lock().unwrap();
inner_ref
.app_address
.clone()
.unwrap_or_default()
.parse::<Uri>()
}
fn reset_app_address(&self) -> Result<(), Self::Error> {
let mut inner_ref = self.0.lock().unwrap();
let _ = inner_ref.app_address.take();
Ok(())
}
fn has_app_address(&self) -> bool {
let inner_ref = self.0.lock().unwrap();
inner_ref.app_address.is_some()
}
}

impl WifiCredentialStorage for RAMStorage {
Expand Down
18 changes: 15 additions & 3 deletions micro-rdk/src/esp32/nvs_storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,14 +145,27 @@ impl RobotConfigurationStorage for NVSStorage {
fn get_robot_credentials(&self) -> Result<RobotCredentials, Self::Error> {
let robot_secret = self.get_string(NVS_ROBOT_SECRET_KEY)?;
let robot_id = self.get_string(NVS_ROBOT_ID_KEY)?;
let app_address = self.get_string(NVS_ROBOT_APP_ADDRESS)?;
Ok(RobotCredentials {
robot_secret,
robot_id,
app_address: app_address.parse::<Uri>()?,
})
}

fn get_app_address(&self) -> Result<Uri, Self::Error> {
Ok(self.get_string(NVS_ROBOT_APP_ADDRESS)?.parse::<Uri>()?)
}

fn has_app_address(&self) -> bool {
self.has_string(NVS_ROBOT_APP_ADDRESS).unwrap_or(false)
}

fn store_app_address(&self, uri: &str) -> Result<(), Self::Error> {
self.set_string(NVS_ROBOT_APP_ADDRESS, uri)
}
fn reset_app_address(&self) -> Result<(), Self::Error> {
self.erase_key(NVS_ROBOT_APP_ADDRESS)
}

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)?;
Expand All @@ -163,7 +176,6 @@ impl RobotConfigurationStorage for NVSStorage {
fn reset_robot_credentials(&self) -> Result<(), Self::Error> {
self.erase_key(NVS_ROBOT_SECRET_KEY)?;
self.erase_key(NVS_ROBOT_ID_KEY)?;
self.erase_key(NVS_ROBOT_APP_ADDRESS)?;
Ok(())
}

Expand Down
7 changes: 4 additions & 3 deletions templates/project/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,19 +83,20 @@ fn main() {
if !storage.has_robot_configuration() {
// check if any were statically compiled
// TODO(RSDK-9148): update with app address storage logic when version is incremented
if ROBOT_ID.is_some() && ROBOT_SECRET.is_some() {
if ROBOT_ID.is_some() && ROBOT_SECRET.is_some() && ROBOT_APP_ADDRESS.is_some() {
log::info!("Storing static values from build time robot configuration to NVS");
storage
.store_robot_credentials(
RobotCredentials::new(
ROBOT_ID.unwrap().to_string(),
ROBOT_SECRET.unwrap().to_string(),
ROBOT_APP_ADDRESS.unwrap().to_string(),
)
.expect("Failed to parse app address")
.into(),
)
.expect("Failed to store robot credentials to NVS");
storage
.store_app_address(ROBOT_APP_ADDRESS.unwrap())
.expect("Failed to store app address to NVS")
}
}

Expand Down

0 comments on commit 0aca25f

Please sign in to comment.