rework documentation and error messages

crates/nao/src/lib.rs

@@ -3,6 +3,7 @@ use std::{
     net::Ipv4Addr,
     path::Path,
     process::Stdio,
+    time::Duration,
 };
 
 use color_eyre::{
@@ -16,7 +17,7 @@ use tokio::{
     select,
 };
 
-pub const PING_TIMEOUT_SECONDS: u32 = 2;
+const PING_TIMEOUT: Duration = Duration::from_secs(2);
 
 const NAO_SSH_FLAGS: &[&str] = &[
     "-lnao",
@@ -47,17 +48,17 @@ pub struct Nao {
 }
 
 impl Nao {
-    pub fn new(host: Ipv4Addr) -> Self {
-        Self { address: host }
+    pub fn new(address: Ipv4Addr) -> Self {
+        Self { address }
     }
 
     pub async fn try_new_with_ping(host: Ipv4Addr) -> Result<Self> {
-        Self::try_new_with_ping_and_arguments(host, PING_TIMEOUT_SECONDS).await
+        Self::try_new_with_ping_and_arguments(host, PING_TIMEOUT).await
     }
 
     pub async fn try_new_with_ping_and_arguments(
         host: Ipv4Addr,
-        timeout_seconds: u32,
+        timeout: Duration,
     ) -> Result<Self> {
         #[cfg(target_os = "macos")]
         const TIMEOUT_FLAG: &str = "-t";
@@ -68,7 +69,7 @@ impl Nao {
             .arg("-c")
             .arg("1")
             .arg(TIMEOUT_FLAG)
-            .arg(timeout_seconds.to_string())
+            .arg(timeout.as_secs().to_string())
             .arg(host.to_string())
             .output()
             .await

crates/repository/src/cargo.rs

@@ -194,7 +194,7 @@ impl<'a> CargoCommand<'a> {
                     repository_root=self.repository_root.to_str().wrap_err("failed to convert repository root to string")?,
                     cargo_home=cargo_home.to_str().wrap_err("failed to convert cargo home to string")?,
                     command=self.sub_command,
-)
+                )
             }
         };
         Ok(shell_command)

crates/repository/src/communication.rs

@@ -1,15 +1,14 @@
 use std::path::Path;
 
-use color_eyre::Result;
+use color_eyre::{eyre::Context, Result};
 use serde_json::Value;
 
 use crate::modify_json::modify_json_inplace;
 
-/// Sets the communication address in the framework.json file.
-///
-/// This function takes a boolean value to enable or disable communication. It reads the
-/// framework.json file, updates the communication address field, and writes the updated JSON back.
-pub async fn set_communication(enable: bool, repository_root: impl AsRef<Path>) -> Result<()> {
+pub async fn configure_communication(
+    enable: bool,
+    repository_root: impl AsRef<Path>,
+) -> Result<()> {
     let framework_json = repository_root
         .as_ref()
         .join("etc/parameters/framework.json");
@@ -20,11 +19,17 @@ pub async fn set_communication(enable: bool, repository_root: impl AsRef<Path>)
         Value::Null
     };
 
-    modify_json_inplace(framework_json, |mut hardware_json: Value| {
+    modify_json_inplace(&framework_json, |mut hardware_json: Value| {
         hardware_json["communication_addresses"] = address;
         hardware_json
     })
-    .await?;
+    .await
+    .wrap_err_with(|| {
+        format!(
+            "failed to configure communication address in {}",
+            framework_json.display()
+        )
+    })?;
 
     Ok(())
 }

crates/repository/src/configuration.rs

@@ -10,17 +10,19 @@ pub struct Configuration {
     pub sdk_version: String,
 }
 
-pub async fn get_os_version(repository_root: impl AsRef<Path>) -> Result<String> {
-    let hulk = find_and_parse_hulk_toml(repository_root).await?;
+/// Get the OS version configured in the `hulk.toml`.
+pub async fn read_os_version(repository_root: impl AsRef<Path>) -> Result<String> {
+    let hulk = read_hulk_toml(repository_root).await?;
     Ok(hulk.os_version)
 }
 
-pub async fn get_sdk_version(repository_root: impl AsRef<Path>) -> Result<String> {
-    let hulk = find_and_parse_hulk_toml(repository_root).await?;
+/// Get the SDK version configured in the `hulk.toml`.
+pub async fn read_sdk_version(repository_root: impl AsRef<Path>) -> Result<String> {
+    let hulk = read_hulk_toml(repository_root).await?;
     Ok(hulk.sdk_version)
 }
 
-pub async fn find_and_parse_hulk_toml(repository_root: impl AsRef<Path>) -> Result<Configuration> {
+pub async fn read_hulk_toml(repository_root: impl AsRef<Path>) -> Result<Configuration> {
     let hulk_toml = repository_root.as_ref().join("hulk.toml");
     let hulk_toml = read_to_string(hulk_toml)
         .await

crates/repository/src/inspect_version.rs

@@ -9,71 +9,33 @@ use serde::Deserialize;
 struct Cargo {
     package: Package,
 }
+
 #[derive(Deserialize, Debug)]
 struct Package {
     version: String,
 }
 
-/// Inspects and returns the version of a Cargo package from its `Cargo.toml` file.
-///
-/// This function takes a path to a `Cargo.toml` file, reads the file,
-/// parses the contents to extract the package version, and returns the version.
-///
-/// # Errors
-///
-/// This function will return an error if:
-///
-/// * The `Cargo.toml` file cannot be read.
-/// * The contents of the `Cargo.toml` file cannot be parsed.
-/// * The version string extracted from the `Cargo.toml` file cannot be parsed into a `Version` object.
-///
-/// # Example
-///
-/// ```no_run
-/// let version = inspect_version(Path::new("path/to/Cargo.toml")).expect("failed to inspect version");
-/// println!("Package version: {}", version);
-/// ```
-pub fn inspect_version(path: impl AsRef<Path>) -> Result<Version> {
-    let path = path.as_ref();
-    let cargo_toml_text = read_to_string(path)
-        .wrap_err_with(|| format!("failed to load Cargo.toml at {}", path.display()))?;
-    let cargo_toml: Cargo = toml::from_str(&cargo_toml_text)
-        .wrap_err_with(|| format!("failed to parse package version from {}", path.display()))?;
-    let version = Version::parse(&cargo_toml.package.version).wrap_err_with(|| {
-        format!(
-            "failed to parse package version '{}' from {}",
-            cargo_toml.package.version,
-            path.display()
-        )
-    })?;
+/// Inspects and returns the version of a package from its `Cargo.toml` file.
+pub fn inspect_version(toml_path: impl AsRef<Path>) -> Result<Version> {
+    let toml_path = toml_path.as_ref();
+    let cargo_toml_text = read_to_string(toml_path).wrap_err("failed to read file")?;
+    let cargo_toml: Cargo = toml::from_str(&cargo_toml_text).wrap_err("failed to parse content")?;
+    let raw_version = &cargo_toml.package.version;
+    let version = Version::parse(raw_version)
+        .wrap_err_with(|| format!("failed to parse version '{raw_version}' as SemVer"))?;
     Ok(version)
 }
 
-/// Checks if there is a new version of the crate available.
-///
-/// This function takes the current version of a package and the path to a `Cargo.toml` file,
-/// reads and parses the version from the `Cargo.toml` file, and compares it with the current version.
-/// If the version in the `Cargo.toml` file is newer, it prints a message indicating that a new version
-/// is available and provides the command to install the new version.
-///
-/// # Errors
-///
-/// This function will return an error if:
-///
-/// * The `own_version` string cannot be parsed into a `Version` object.
-/// * The `Cargo.toml` file cannot be read or parsed to extract the package version.
-///
-/// # Example
-///
-/// ```no_run
-/// let current_version = "1.0.0";
-/// let cargo_toml_path = Path::new("path/to/Cargo.toml");
-///
-/// check_for_update(current_version, &cargo_toml_path).expect("failed to check for update");
-/// ```
+/// Checks whether the package has a newer version than the provided version.
 pub fn check_for_update(own_version: &str, cargo_toml: impl AsRef<Path>) -> Result<()> {
-    let own_version = Version::parse(own_version)?;
-    let cargo_toml_version = inspect_version(&cargo_toml)?;
+    let own_version = Version::parse(own_version)
+        .wrap_err_with(|| format!("failed to parse own version '{own_version}' as SemVer"))?;
+    let cargo_toml_version = inspect_version(&cargo_toml).wrap_err_with(|| {
+        format!(
+            "failed to inspect version of package at {}",
+            cargo_toml.as_ref().display()
+        )
+    })?;
     if own_version < cargo_toml_version {
         let crate_path = cargo_toml.as_ref().parent().unwrap();
         warn!(

crates/repository/src/lib.rs

@@ -1,3 +1,8 @@
+//! Tools and utilities for managing development workflows in the repository.
+//!
+//! This crate simplifies tasks like building for specific targets, handling SDKs, and setting up
+//! configurations, making it easier to develop, configure, and deploy for NAO robots.
+
 pub mod cargo;
 pub mod communication;
 pub mod configuration;

crates/repository/src/location.rs

@@ -1,8 +1,4 @@
-use std::{
-    collections::{HashMap, HashSet},
-    io::ErrorKind,
-    path::Path,
-};
+use std::{io::ErrorKind, path::Path};
 
 use color_eyre::{
     eyre::{bail, eyre, Context},
@@ -15,9 +11,9 @@ use tokio::{
     io,
 };
 
-pub async fn get_configured_locations(
+pub async fn list_configured_locations(
     repository_root: impl AsRef<Path>,
-) -> Result<HashMap<String, Option<String>>> {
+) -> Result<Vec<(String, Option<String>)>> {
     let parameters_root = &repository_root.as_ref().join("etc/parameters");
     let results: Vec<_> = [
         "nao_location",
@@ -66,8 +62,13 @@ pub async fn set_location(
     repository_root: impl AsRef<Path>,
 ) -> Result<()> {
     let parameters_root = repository_root.as_ref().join("etc/parameters");
-    if !try_exists(parameters_root.join(location)).await? {
-        let location_set = list_available_locations(&repository_root).await?;
+    if !try_exists(parameters_root.join(location))
+        .await
+        .wrap_err_with(|| format!("failed checking if location '{location}' exists"))?
+    {
+        let location_set = list_available_locations(&repository_root)
+            .await
+            .unwrap_or_default();
         let available_locations: String = intersperse(
             location_set
                 .into_iter()
@@ -76,29 +77,28 @@ pub async fn set_location(
         )
         .collect();
         bail!(
-            "location {location} does not exist. \navailable locations are:\n{available_locations}"
+            "location {location} does not exist.\navailable locations are:\n{available_locations}"
         );
     }
     let target_location = parameters_root.join(format!("{target}_location"));
     let _ = remove_file(&target_location).await;
-    symlink(location, &target_location)
-            .await
-            .wrap_err_with(|| {
-                format!("failed creating symlink named {target_location:?} pointing to {location:?}, does the location exist?")
-            })
+    symlink(location, &target_location).await.wrap_err_with(|| {
+        format!(
+            "failed creating symlink named {target_location} pointing to {location}",
+            target_location = target_location.display()
+        )
+    })
 }
 
-pub async fn list_available_locations(
-    repository_root: impl AsRef<Path>,
-) -> Result<HashSet<String>> {
+pub async fn list_available_locations(repository_root: impl AsRef<Path>) -> Result<Vec<String>> {
     let parameters_root = repository_root.as_ref().join("etc/parameters");
     let mut locations = read_dir(parameters_root)
         .await
         .wrap_err("failed to read parameters directory")?;
-    let mut results = HashSet::new();
+    let mut results = Vec::new();
     while let Ok(Some(entry)) = locations.next_entry().await {
         if entry.path().is_dir() && !entry.path().is_symlink() {
-            results.insert(
+            results.push(
                 entry
                     .path()
                     .file_name()

crates/repository/src/player_number.rs

@@ -8,10 +8,7 @@ use parameters::{
 use spl_network_messages::PlayerNumber;
 use types::hardware::Ids;
 
-/// Sets the player number in the parameters directory.
-///
-/// This function takes a head ID of a robot and a player number, and writes it to the parameters.
-pub async fn set_player_number(
+pub async fn configure_player_number(
     head_id: &str,
     player_number: PlayerNumber,
     repository_root: impl AsRef<Path>,
@@ -35,5 +32,6 @@ pub async fn set_player_number(
             head_id: head_id.to_string(),
         },
     )
-    .wrap_err("failed to serialize parameters directory")
+    .wrap_err("failed to serialize parameters directory")?;
+    Ok(())
 }

crates/repository/src/recording.rs

@@ -5,7 +5,7 @@ use serde_json::{to_value, Value};
 
 use crate::modify_json::modify_json_inplace;
 
-pub async fn set_recording_intervals(
+pub async fn configure_recording_intervals(
     recording_intervals: HashMap<String, usize>,
     repository_root: impl AsRef<Path>,
 ) -> Result<()> {
@@ -15,11 +15,17 @@ pub async fn set_recording_intervals(
     let serialized_intervals =
         to_value(recording_intervals).wrap_err("failed to convert recording intervals to JSON")?;
 
-    modify_json_inplace(framework_json, |mut hardware_json: Value| {
+    modify_json_inplace(&framework_json, |mut hardware_json: Value| {
         hardware_json["recording_intervals"] = serialized_intervals;
         hardware_json
     })
-    .await?;
+    .await
+    .wrap_err_with(|| {
+        format!(
+            "failed to configure recording intervals in {}",
+            framework_json.display()
+        )
+    })?;
 
     Ok(())
 }

crates/repository/src/sdk.rs

@@ -67,7 +67,9 @@ async fn download(version: &str, sdk_home: impl AsRef<Path>) -> Result<PathBuf>
         format!("http://bighulk.hulks.dev/sdk/{installer_name}"),
         format!("https://github.com/HULKs/meta-nao/releases/download/{version}/{installer_name}"),
     ];
-    download_with_fallback(urls, &download_path, CONNECT_TIMEOUT).await?;
+    download_with_fallback(urls, &download_path, CONNECT_TIMEOUT)
+        .await
+        .wrap_err("failed to download SDK")?;
 
     set_permissions(&download_path, Permissions::from_mode(0o755))
         .await

crates/repository/src/team.rs

@@ -18,12 +18,12 @@ pub struct Nao {
     pub head_id: String,
 }
 
-pub async fn get_team_configuration(repository_root: impl AsRef<Path>) -> Result<Team> {
+pub async fn read_team_configuration(repository_root: impl AsRef<Path>) -> Result<Team> {
     let team_toml = repository_root.as_ref().join("etc/parameters/team.toml");
 
     let content = read_to_string(&team_toml)
         .await
-        .wrap_err_with(|| format!("failed to open {}", team_toml.display()))?;
+        .wrap_err_with(|| format!("failed to read {}", team_toml.display()))?;
 
     let team = toml::from_str(&content).wrap_err("failed to parse team.toml")?;
     Ok(team)

tools/pepsi/src/aliveness.rs

@@ -14,7 +14,7 @@ use aliveness::{
     AlivenessError, AlivenessState, Battery, JointsArray,
 };
 use argument_parsers::NaoAddress;
-use repository::configuration::get_os_version;
+use repository::configuration::read_os_version;
 
 #[derive(Args)]
 pub struct Arguments {
@@ -52,7 +52,7 @@ pub async fn aliveness(
         print_verbose(&states);
     } else {
         let expected_os_version = match repository_root {
-            Ok(repository_root) => match get_os_version(repository_root).await {
+            Ok(repository_root) => match read_os_version(repository_root).await {
                 Ok(version) => Some(version),
                 Err(error) => {
                     error!("{error:#?}");