From 9f253efd6b1559491adb0509d5cdd8285e5dea79 Mon Sep 17 00:00:00 2001 From: alberto tirla Date: Thu, 14 Mar 2024 18:10:31 +0200 Subject: [PATCH 01/19] add preliminary support for more advanced configuration options in odilia this is currently using figment as the configuration library, other options include config and simply serde-toml this is not yet complete, as huge refactorings are about to take place --- Cargo.lock | 146 +++++++++++++++++++++++++++++++--- common/Cargo.toml | 2 +- common/src/errors.rs | 10 +-- common/src/settings/log.rs | 6 +- common/src/settings/mod.rs | 21 +++-- common/src/settings/speech.rs | 6 +- 6 files changed, 162 insertions(+), 29 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 79044f9e..f3e7e7a0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -343,6 +343,15 @@ dependencies = [ "syn 2.0.52", ] +[[package]] +name = "atomic" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8d818003e740b63afc82337e3160717f4f63078720a810b7b903e70a5d1d2994" +dependencies = [ + "bytemuck", +] + [[package]] name = "atomic-waker" version = "1.1.2" @@ -511,6 +520,12 @@ version = "3.15.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7ff69b9dd49fd426c69a0db9fc04dd934cdb6645ff000864d98f7e2af8830eaa" +[[package]] +name = "bytemuck" +version = "1.15.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5d6d68c57235a3a081186990eca2867354726650f42f7516ca50c28d6281fd15" + [[package]] name = "byteorder" version = "1.5.0" @@ -958,6 +973,22 @@ version = "2.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "25cbce373ec4653f1a01a31e8a5e5ec0c622dc27ff9c4e6606eefef5cbbed4a5" +[[package]] +name = "figment" +version = "0.10.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2b6e5bc7bd59d60d0d45a6ccab6cf0f4ce28698fb4e81e750ddf229c9b824026" +dependencies = [ + "atomic", + "parking_lot", + "pear", + "serde", + "tempfile", + "toml", + "uncased", + "version_check", +] + [[package]] name = "futures" version = "0.3.30" @@ -1202,6 +1233,12 @@ dependencies = [ "hashbrown 0.14.3", ] +[[package]] +name = "inlinable_string" +version = "0.1.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c8fae54786f62fb2918dcfae3d568594e50eb9b5c25bf04371af6fe7516452fb" + [[package]] name = "instant" version = "0.1.12" @@ -1566,11 +1603,11 @@ dependencies = [ "atspi", "atspi-common", "bitflags 1.3.2", + "figment", "serde", "serde_plain", "smartstring", "thiserror", - "tini", "zbus", ] @@ -1679,6 +1716,29 @@ dependencies = [ "windows-targets 0.48.5", ] +[[package]] +name = "pear" +version = "0.2.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4ccca0f6c17acc81df8e242ed473ec144cbf5c98037e69aa6d144780aad103c8" +dependencies = [ + "inlinable_string", + "pear_codegen", + "yansi", +] + +[[package]] +name = "pear_codegen" +version = "0.2.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2e22670e8eb757cff11d6c199ca7b987f352f0346e0be4dd23869ec72cb53c77" +dependencies = [ + "proc-macro2", + "proc-macro2-diagnostics", + "quote", + "syn 2.0.52", +] + [[package]] name = "pin-project-lite" version = "0.2.13" @@ -1779,7 +1839,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7f4c021e1093a56626774e81216a4ce732a735e5bad4868a03f3ed65ca0c3919" dependencies = [ "once_cell", - "toml_edit", + "toml_edit 0.19.15", ] [[package]] @@ -1791,6 +1851,19 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "proc-macro2-diagnostics" +version = "0.10.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "af066a9c399a26e020ada66a034357a868728e72cd426f3adcd35f80d88d88c8" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.52", + "version_check", + "yansi", +] + [[package]] name = "quick-xml" version = "0.30.0" @@ -2046,6 +2119,15 @@ dependencies = [ "syn 2.0.52", ] +[[package]] +name = "serde_spanned" +version = "0.6.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "eb3622f419d1296904700073ea6cc23ad690adbd66f13ea683df73298736f0c1" +dependencies = [ + "serde", +] + [[package]] name = "sha1" version = "0.10.6" @@ -2293,12 +2375,6 @@ version = "0.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ef927ca75afb808a4d64dd374f00a2adf8d0fcff8e7b184af886c3c87ec4a3f3" -[[package]] -name = "tini" -version = "1.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e004df4c5f0805eb5f55883204a514cfa43a6d924741be29e871753a53d5565a" - [[package]] name = "tinytemplate" version = "1.2.1" @@ -2378,11 +2454,26 @@ dependencies = [ "tokio", ] +[[package]] +name = "toml" +version = "0.8.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9a9aad4a3066010876e8dcf5a8a06e70a558751117a145c6ce2b82c2e2054290" +dependencies = [ + "serde", + "serde_spanned", + "toml_datetime", + "toml_edit 0.22.7", +] + [[package]] name = "toml_datetime" version = "0.6.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3550f4e9685620ac18a50ed434eb3aec30db8ba93b0287467bca5826ea25baf1" +dependencies = [ + "serde", +] [[package]] name = "toml_edit" @@ -2392,7 +2483,20 @@ checksum = "1b5bb770da30e5cbfde35a2d7b9b8a2c4b8ef89548a7a6aeab5c9a576e3e7421" dependencies = [ "indexmap 2.2.5", "toml_datetime", - "winnow", + "winnow 0.5.40", +] + +[[package]] +name = "toml_edit" +version = "0.22.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "18769cd1cec395d70860ceb4d932812a0b4d06b1a4bb336745a4d21b9496e992" +dependencies = [ + "indexmap 2.2.5", + "serde", + "serde_spanned", + "toml_datetime", + "winnow 0.6.5", ] [[package]] @@ -2493,6 +2597,15 @@ dependencies = [ "winapi", ] +[[package]] +name = "uncased" +version = "0.9.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e1b88fcfe09e89d3866a5c11019378088af2d24c3fbd4f0543f96b479ec90697" +dependencies = [ + "version_check", +] + [[package]] name = "unicode-ident" version = "1.0.12" @@ -2812,6 +2925,15 @@ dependencies = [ "memchr", ] +[[package]] +name = "winnow" +version = "0.6.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dffa400e67ed5a4dd237983829e66475f0a4a26938c4b04c21baede6262215b8" +dependencies = [ + "memchr", +] + [[package]] name = "xdg" version = "2.5.2" @@ -2834,6 +2956,12 @@ version = "0.8.19" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0fcb9cbac069e033553e8bb871be2fbdffcab578eb25bd0f7c508cedc6dcd75a" +[[package]] +name = "yansi" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cfe53a6657fd280eaa890a3bc59152892ffa3e30101319d168b781ed6529b049" + [[package]] name = "zbus" version = "3.15.2" diff --git a/common/Cargo.toml b/common/Cargo.toml index 5dd467c3..4b7e8095 100644 --- a/common/Cargo.toml +++ b/common/Cargo.toml @@ -18,6 +18,6 @@ bitflags = "1.3.2" serde = "1.0.147" smartstring = "1.0.1" thiserror = "1.0.37" -tini = "^1.3.0" zbus.workspace = true serde_plain.workspace = true +figment = { version = "0.10.14", features = ["toml", "test", "env"] } diff --git a/common/src/errors.rs b/common/src/errors.rs index ae6ae033..271ecbba 100644 --- a/common/src/errors.rs +++ b/common/src/errors.rs @@ -23,19 +23,19 @@ pub enum OdiliaError { } #[derive(Debug)] pub enum ConfigError { - Tini(tini::Error), + Figment(figment::Error), ValueNotFound, PathNotFound, } -impl From for ConfigError { - fn from(t_err: tini::Error) -> Self { - Self::Tini(t_err) +impl From for ConfigError { + fn from(t_err: figment::Error) -> Self { + Self::Figment(t_err) } } impl std::fmt::Display for ConfigError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - Self::Tini(t) => t.fmt(f), + Self::Figment(t) => t.fmt(f), Self::ValueNotFound => f.write_str("Vlaue not found in config file."), Self::PathNotFound => { f.write_str("The path for the config file was not found.") diff --git a/common/src/settings/log.rs b/common/src/settings/log.rs index 6bf3787c..a4a1ac2f 100644 --- a/common/src/settings/log.rs +++ b/common/src/settings/log.rs @@ -5,8 +5,8 @@ use serde::{Deserialize, Serialize}; pub struct LogSettings { level: String, } -impl LogSettings { - pub fn new(level: String) -> Self { - Self { level } +impl Default for LogSettings { + fn default() -> Self { + Self { level: "info".to_owned() } } } diff --git a/common/src/settings/mod.rs b/common/src/settings/mod.rs index 1d89441f..7aa07375 100644 --- a/common/src/settings/mod.rs +++ b/common/src/settings/mod.rs @@ -1,17 +1,22 @@ mod log; mod speech; + use log::LogSettings; use speech::SpeechSettings; use serde::{Deserialize, Serialize}; -use tini::Ini; + +use figment::{ + providers::{Env, Format, Serialized, Toml}, + Figment, +}; use crate::errors::ConfigError; ///type representing a *read-only* view of the odilia screenreader configuration /// this type should only be obtained as a result of parsing odilia's configuration files, as it containes types for each section responsible for controlling various parts of the screenreader /// the only way this config should change is if the configuration file changes, in which case the entire view will be replaced to reflect the fact -#[derive(Debug, Serialize, Deserialize)] +#[derive(Debug, Serialize, Deserialize, Default)] pub struct ApplicationConfig { speech: SpeechSettings, log: LogSettings, @@ -24,12 +29,12 @@ impl ApplicationConfig { /// /// This can return `Err(_)` if the path doesn't exist, or if not all the key/value pairs are defined. pub fn new(path: &str) -> Result { - let ini = Ini::from_file(path)?; - let rate: i32 = ini.get("speech", "rate").ok_or(ConfigError::ValueNotFound)?; - let level: String = ini.get("log", "level").ok_or(ConfigError::ValueNotFound)?; - let speech = SpeechSettings::new(rate); - let log = LogSettings::new(level); - Ok(Self { speech, log }) + let config: Self = + Figment::from(Serialized::defaults(ApplicationConfig::default())) + .merge(Toml::file(path)) + .merge(Env::prefixed("ODILIA_")) + .extract()?; + Ok(config) } #[must_use] diff --git a/common/src/settings/speech.rs b/common/src/settings/speech.rs index da19b376..e065fab7 100644 --- a/common/src/settings/speech.rs +++ b/common/src/settings/speech.rs @@ -5,8 +5,8 @@ use serde::{Deserialize, Serialize}; pub struct SpeechSettings { pub rate: i32, } -impl SpeechSettings { - pub fn new(rate: i32) -> Self { - Self { rate } +impl Default for SpeechSettings { + fn default() -> Self { + Self { rate: 50 } } } From d59c33833e10dd1b4cca5136ae4fe1e2c0764f44 Mon Sep 17 00:00:00 2001 From: alberto tirla Date: Thu, 14 Mar 2024 18:37:42 +0200 Subject: [PATCH 02/19] refactor state a bit, to create config.toml from scratch based on default values --- Cargo.lock | 5 +++-- odilia/Cargo.toml | 1 + odilia/src/state.rs | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f3e7e7a0..955037c2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1563,6 +1563,7 @@ dependencies = [ "tokio", "tokio-test", "tokio-util", + "toml", "tracing", "tracing-error", "tracing-log", @@ -2456,9 +2457,9 @@ dependencies = [ [[package]] name = "toml" -version = "0.8.10" +version = "0.8.11" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9a9aad4a3066010876e8dcf5a8a06e70a558751117a145c6ce2b82c2e2054290" +checksum = "af06656561d28735e9c1cd63dfd57132c8155426aa6af24f36a00a351f88c48e" dependencies = [ "serde", "serde_spanned", diff --git a/odilia/Cargo.toml b/odilia/Cargo.toml index ef339b56..2bc03f8e 100644 --- a/odilia/Cargo.toml +++ b/odilia/Cargo.toml @@ -56,6 +56,7 @@ zbus.workspace = true odilia-notify = { version = "0.1.0", path = "../odilia-notify" } clap = { version = "4.5.1", features = ["derive"] } tokio-util.workspace=true +toml = "0.8.11" [dev-dependencies] lazy_static = "1.4.0" diff --git a/odilia/src/state.rs b/odilia/src/state.rs index 51237248..a2b1de51 100644 --- a/odilia/src/state.rs +++ b/odilia/src/state.rs @@ -100,7 +100,8 @@ impl ScreenReaderState { ); if !config_path.exists() { - fs::write(&config_path, include_str!("../config.toml")) + let toml = toml::to_string(&ApplicationConfig::default())?; + fs::write(&config_path, toml) .expect("Unable to copy default config file."); } config_path.to_str().ok_or(ConfigError::PathNotFound)?.to_owned() From 4cf59483bbf17b4aae34f72d19a4035f3b01b2c9 Mon Sep 17 00:00:00 2001 From: alberto tirla Date: Sat, 16 Mar 2024 14:43:03 +0200 Subject: [PATCH 03/19] refactor State::new to use figment directly --- Cargo.lock | 7 ++- common/Cargo.toml | 2 +- common/src/settings/mod.rs | 21 --------- odilia/Cargo.toml | 1 + odilia/src/state.rs | 89 +++++++++++++++----------------------- 5 files changed, 39 insertions(+), 81 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 955037c2..0f007f60 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -975,15 +975,13 @@ checksum = "25cbce373ec4653f1a01a31e8a5e5ec0c622dc27ff9c4e6606eefef5cbbed4a5" [[package]] name = "figment" -version = "0.10.14" +version = "0.10.15" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2b6e5bc7bd59d60d0d45a6ccab6cf0f4ce28698fb4e81e750ddf229c9b824026" +checksum = "7270677e7067213e04f323b55084586195f18308cd7546cfac9f873344ccceb6" dependencies = [ "atomic", - "parking_lot", "pear", "serde", - "tempfile", "toml", "uncased", "version_check", @@ -1550,6 +1548,7 @@ dependencies = [ "circular-queue", "clap 4.5.2", "eyre", + "figment", "futures", "lazy_static", "odilia-cache", diff --git a/common/Cargo.toml b/common/Cargo.toml index 4b7e8095..65ca163f 100644 --- a/common/Cargo.toml +++ b/common/Cargo.toml @@ -20,4 +20,4 @@ smartstring = "1.0.1" thiserror = "1.0.37" zbus.workspace = true serde_plain.workspace = true -figment = { version = "0.10.14", features = ["toml", "test", "env"] } +figment = "0.10.15" diff --git a/common/src/settings/mod.rs b/common/src/settings/mod.rs index 7aa07375..766388a0 100644 --- a/common/src/settings/mod.rs +++ b/common/src/settings/mod.rs @@ -6,13 +6,6 @@ use speech::SpeechSettings; use serde::{Deserialize, Serialize}; -use figment::{ - providers::{Env, Format, Serialized, Toml}, - Figment, -}; - -use crate::errors::ConfigError; - ///type representing a *read-only* view of the odilia screenreader configuration /// this type should only be obtained as a result of parsing odilia's configuration files, as it containes types for each section responsible for controlling various parts of the screenreader /// the only way this config should change is if the configuration file changes, in which case the entire view will be replaced to reflect the fact @@ -23,20 +16,6 @@ pub struct ApplicationConfig { } impl ApplicationConfig { - /// Opens a new config file with a certain path. - /// - /// # Errors - /// - /// This can return `Err(_)` if the path doesn't exist, or if not all the key/value pairs are defined. - pub fn new(path: &str) -> Result { - let config: Self = - Figment::from(Serialized::defaults(ApplicationConfig::default())) - .merge(Toml::file(path)) - .merge(Env::prefixed("ODILIA_")) - .extract()?; - Ok(config) - } - #[must_use] pub fn log(&self) -> &LogSettings { &self.log diff --git a/odilia/Cargo.toml b/odilia/Cargo.toml index 2bc03f8e..1e2c0881 100644 --- a/odilia/Cargo.toml +++ b/odilia/Cargo.toml @@ -57,6 +57,7 @@ odilia-notify = { version = "0.1.0", path = "../odilia-notify" } clap = { version = "4.5.1", features = ["derive"] } tokio-util.workspace=true toml = "0.8.11" +figment = { version = "0.10.14", features = ["env", "toml"] } [dev-dependencies] lazy_static = "1.4.0" diff --git a/odilia/src/state.rs b/odilia/src/state.rs index a2b1de51..aa8e2dad 100644 --- a/odilia/src/state.rs +++ b/odilia/src/state.rs @@ -1,3 +1,7 @@ +use figment::{ + providers::{Env, Format, Serialized, Toml}, + Figment, +}; use std::{fs, sync::atomic::AtomicI32}; use circular_queue::CircularQueue; @@ -16,11 +20,8 @@ use atspi_connection::AccessibilityConnection; use atspi_proxies::{accessible::AccessibleProxy, cache::CacheProxy}; use odilia_cache::{AccessiblePrimitive, Cache, CacheItem}; use odilia_common::{ - errors::{CacheError, ConfigError}, - modes::ScreenReaderMode, - settings::ApplicationConfig, - types::TextSelectionArea, - Result as OdiliaResult, + errors::CacheError, modes::ScreenReaderMode, settings::ApplicationConfig, + types::TextSelectionArea, Result as OdiliaResult, }; use std::sync::Arc; @@ -37,13 +38,6 @@ pub struct ScreenReaderState { pub cache: Arc, } -enum ConfigType { - CliOverride, - XDGConfigHome, - Etc, - CreateDefault, -} - impl ScreenReaderState { #[tracing::instrument(skip_all)] pub async fn new( @@ -65,54 +59,39 @@ impl ScreenReaderState { tracing::debug!("Reading configuration"); - // In order of prioritization, do configuration via cli, then XDG_CONFIG_HOME, then /etc/, + // In order of prioritization, do environment variables, configuration via cli, then XDG_CONFIG_HOME, then /etc/odilia, // Otherwise create it in XDG_CONFIG_HOME - let config_type = if config_override.is_some() { - ConfigType::CliOverride - - // First check makes sure unwrap is safe - } else if xdg::BaseDirectories::with_prefix("odilia").is_ok() - && xdg::BaseDirectories::with_prefix("odilia") - .expect("This error should never occur") - .find_config_file("config.toml") - .is_some() - { - ConfigType::XDGConfigHome - } else if std::path::Path::new("/etc/odilia/config.toml").exists() { - ConfigType::Etc + //default configuration first, because that doesn't affect the priority outlined above + let figment = Figment::from(Serialized::defaults(ApplicationConfig::default())); + //environment variables + let figment = figment.merge(Env::prefixed("ODILIA_")); + //cli override, if applicable + let figment = if let Some(path) = config_override { + figment.merge(Toml::file(path)) } else { - ConfigType::CreateDefault + figment }; + //create a config.toml file in `XDG_CONFIG_HOME`, to make it possible for the user to edit the default values, if it doesn't exist already + let xdg_dirs = xdg::BaseDirectories::with_prefix("odilia").expect( + "unable to find the odilia config directory according to the xdg dirs specification", + ); - let config_path = match config_type { - ConfigType::CliOverride => config_override - .expect("Config override was provided but is None") - .to_str() - .ok_or(ConfigError::PathNotFound)? - .to_owned(), - ConfigType::XDGConfigHome | ConfigType::CreateDefault => { - let xdg_dirs = xdg::BaseDirectories::with_prefix("odilia").expect( - "unable to find the odilia config directory according to the xdg dirs specification", - ); - - let config_path = xdg_dirs.place_config_file("config.toml").expect( - "unable to place configuration file. Maybe your system is readonly?", - ); - - if !config_path.exists() { - let toml = toml::to_string(&ApplicationConfig::default())?; - fs::write(&config_path, toml) - .expect("Unable to copy default config file."); - } - config_path.to_str().ok_or(ConfigError::PathNotFound)?.to_owned() - } - ConfigType::Etc => "/etc/odilia/config.toml".to_owned(), - }; - - tracing::debug!(path=%config_path, "loading configuration file"); - let config = ApplicationConfig::new(&config_path) - .wrap_err("unable to load configuration file")?; + let config_path = xdg_dirs.place_config_file("config.toml").expect( + "unable to place configuration file. Maybe your system is readonly?", + ); + if !config_path.exists() { + let toml = toml::to_string(&ApplicationConfig::default())?; + fs::write(&config_path, toml) + .expect("Unable to create default config file."); + } + //next, the xdg configuration + let figment = figment + .merge(Toml::file(&config_path)) + //last, the configuration system wide, in /etc/odilia/config.toml + .merge(Toml::file("/etc/odilia/config.toml")); + //realise the configuration and freeze it into place + let config: ApplicationConfig = figment.extract()?; tracing::debug!("configuration loaded successfully"); let previous_caret_position = AtomicI32::new(0); From dc5e613a037a1370cef4a290f3121a9c30a9d4a6 Mon Sep 17 00:00:00 2001 From: alberto tirla Date: Sat, 16 Mar 2024 15:04:33 +0200 Subject: [PATCH 04/19] make configuration be working from main, instead of state::new and make state::new accept the configuration structure as a parameter this makes it easier to make, for example, speech have the desired rate from startup, logging be initialised from the config with the desired filter and perhaps a path location, stuff like that --- odilia/src/events/mod.rs | 3 ++- odilia/src/main.rs | 45 ++++++++++++++++++++++++++++++++++++++-- odilia/src/state.rs | 45 ++-------------------------------------- 3 files changed, 47 insertions(+), 46 deletions(-) diff --git a/odilia/src/events/mod.rs b/odilia/src/events/mod.rs index 195269ab..0ffdbf1d 100644 --- a/odilia/src/events/mod.rs +++ b/odilia/src/events/mod.rs @@ -196,6 +196,7 @@ async fn dispatch(state: &ScreenReaderState, event: Event) -> eyre::Result<()> { pub mod dispatch_tests { use crate::ScreenReaderState; use eyre::Context; + use odilia_common::settings::ApplicationConfig; use tokio::sync::mpsc::channel; #[tokio::test] @@ -209,7 +210,7 @@ pub mod dispatch_tests { let (send, _recv) = channel(32); let cache = serde_json::from_str(include_str!("wcag_cache_items.json")) .context("unable to load cache data from json file")?; - let state = ScreenReaderState::new(send, None) + let state = ScreenReaderState::new(send, ApplicationConfig::default()) .await .context("unable to realise screenreader state")?; state.cache diff --git a/odilia/src/main.rs b/odilia/src/main.rs index 1000e56c..77f2d1ef 100644 --- a/odilia/src/main.rs +++ b/odilia/src/main.rs @@ -13,13 +13,18 @@ mod events; mod logging; mod state; -use std::{process::exit, sync::Arc, time::Duration}; +use std::{fs, process::exit, sync::Arc, time::Duration}; use crate::cli::Args; use crate::state::ScreenReaderState; use clap::Parser; use eyre::WrapErr; +use figment::{ + providers::{Env, Format, Serialized, Toml}, + Figment, +}; use futures::{future::FutureExt, StreamExt}; +use odilia_common::settings::ApplicationConfig; use odilia_input::sr_event_receiver; use odilia_notify::listen_to_dbus_notifications; use ssip_client_async::Priority; @@ -85,6 +90,42 @@ async fn main() -> eyre::Result<()> { //initialize a task tracker, which will allow us to wait for all tasks to finish let tracker = TaskTracker::new(); + tracing::debug!("Reading configuration"); + + // In order of prioritization, do environment variables, configuration via cli, then XDG_CONFIG_HOME, then /etc/odilia, + // Otherwise create it in XDG_CONFIG_HOME + //default configuration first, because that doesn't affect the priority outlined above + let figment = Figment::from(Serialized::defaults(ApplicationConfig::default())); + //environment variables + let figment = figment.merge(Env::prefixed("ODILIA_")); + //cli override, if applicable + let figment = if let Some(path) = args.config { + figment.merge(Toml::file(path)) + } else { + figment + }; + //create a config.toml file in `XDG_CONFIG_HOME`, to make it possible for the user to edit the default values, if it doesn't exist already + let xdg_dirs = xdg::BaseDirectories::with_prefix("odilia").expect( + "unable to find the odilia config directory according to the xdg dirs specification", + ); + + let config_path = xdg_dirs + .place_config_file("config.toml") + .expect("unable to place configuration file. Maybe your system is readonly?"); + + if !config_path.exists() { + let toml = toml::to_string(&ApplicationConfig::default())?; + fs::write(&config_path, toml).expect("Unable to create default config file."); + } + //next, the xdg configuration + let figment = figment + .merge(Toml::file(&config_path)) + //last, the configuration system wide, in /etc/odilia/config.toml + .merge(Toml::file("/etc/odilia/config.toml")); + //realise the configuration and freeze it into place + let config: ApplicationConfig = figment.extract()?; + tracing::debug!("configuration loaded successfully"); + // Make sure applications with dynamic accessibility support do expose their AT-SPI2 interfaces. if let Err(e) = atspi_connection::set_session_accessibility(true) .instrument(tracing::info_span!("setting accessibility enabled flag")) @@ -101,7 +142,7 @@ async fn main() -> eyre::Result<()> { // Like the channel above, it is very important that this is *never* full, since it can cause deadlocking if the other task sending the request is working with zbus. let (ssip_req_tx, ssip_req_rx) = mpsc::channel::(128); // Initialize state - let state = Arc::new(ScreenReaderState::new(ssip_req_tx, args.config.as_deref()).await?); + let state = Arc::new(ScreenReaderState::new(ssip_req_tx, config).await?); let ssip = odilia_tts::create_ssip_client().await?; if state.say(Priority::Message, "Welcome to Odilia!".to_string()).await { diff --git a/odilia/src/state.rs b/odilia/src/state.rs index aa8e2dad..d12000b9 100644 --- a/odilia/src/state.rs +++ b/odilia/src/state.rs @@ -1,8 +1,4 @@ -use figment::{ - providers::{Env, Format, Serialized, Toml}, - Figment, -}; -use std::{fs, sync::atomic::AtomicI32}; +use std::sync::atomic::AtomicI32; use circular_queue::CircularQueue; use eyre::WrapErr; @@ -42,7 +38,7 @@ impl ScreenReaderState { #[tracing::instrument(skip_all)] pub async fn new( ssip: Sender, - config_override: Option<&std::path::Path>, + config: ApplicationConfig, ) -> eyre::Result { let atspi = AccessibilityConnection::open() .instrument(tracing::info_span!("connecting to at-spi bus")) @@ -57,43 +53,6 @@ impl ScreenReaderState { let mode = Mutex::new(ScreenReaderMode { name: "CommandMode".to_string() }); - tracing::debug!("Reading configuration"); - - // In order of prioritization, do environment variables, configuration via cli, then XDG_CONFIG_HOME, then /etc/odilia, - // Otherwise create it in XDG_CONFIG_HOME - //default configuration first, because that doesn't affect the priority outlined above - let figment = Figment::from(Serialized::defaults(ApplicationConfig::default())); - //environment variables - let figment = figment.merge(Env::prefixed("ODILIA_")); - //cli override, if applicable - let figment = if let Some(path) = config_override { - figment.merge(Toml::file(path)) - } else { - figment - }; - //create a config.toml file in `XDG_CONFIG_HOME`, to make it possible for the user to edit the default values, if it doesn't exist already - let xdg_dirs = xdg::BaseDirectories::with_prefix("odilia").expect( - "unable to find the odilia config directory according to the xdg dirs specification", - ); - - let config_path = xdg_dirs.place_config_file("config.toml").expect( - "unable to place configuration file. Maybe your system is readonly?", - ); - - if !config_path.exists() { - let toml = toml::to_string(&ApplicationConfig::default())?; - fs::write(&config_path, toml) - .expect("Unable to create default config file."); - } - //next, the xdg configuration - let figment = figment - .merge(Toml::file(&config_path)) - //last, the configuration system wide, in /etc/odilia/config.toml - .merge(Toml::file("/etc/odilia/config.toml")); - //realise the configuration and freeze it into place - let config: ApplicationConfig = figment.extract()?; - tracing::debug!("configuration loaded successfully"); - let previous_caret_position = AtomicI32::new(0); let accessible_history = Mutex::new(CircularQueue::with_capacity(16)); let event_history = Mutex::new(CircularQueue::with_capacity(16)); From cff540bf96c1db013b2c626d0a6f77849e2924ce Mon Sep 17 00:00:00 2001 From: alberto tirla Date: Tue, 19 Mar 2024 13:49:45 +0200 Subject: [PATCH 05/19] fix: screenreader applies configuration properly the method join replaced the previously used merge. Apparently, with merge, when conflicts are encountered, it preferes keeping the current value, instead of replacing it. So, config was taken from the default values supplied with the modules, but the values in there were never replaced by those who should have a greatter priority. as a consequence, user defined configuration files wouldn't be applied, and the user would understandably be confused by the results --- odilia/src/main.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/odilia/src/main.rs b/odilia/src/main.rs index 77f2d1ef..37428c1f 100644 --- a/odilia/src/main.rs +++ b/odilia/src/main.rs @@ -95,12 +95,12 @@ async fn main() -> eyre::Result<()> { // In order of prioritization, do environment variables, configuration via cli, then XDG_CONFIG_HOME, then /etc/odilia, // Otherwise create it in XDG_CONFIG_HOME //default configuration first, because that doesn't affect the priority outlined above - let figment = Figment::from(Serialized::defaults(ApplicationConfig::default())); + let figment = Figment::from(Serialized::defaults(ApplicationConfig::default())) //environment variables - let figment = figment.merge(Env::prefixed("ODILIA_")); + .join(Env::prefixed("ODILIA_")); //cli override, if applicable let figment = if let Some(path) = args.config { - figment.merge(Toml::file(path)) + figment.join(Toml::file(path)) } else { figment }; @@ -119,12 +119,12 @@ async fn main() -> eyre::Result<()> { } //next, the xdg configuration let figment = figment - .merge(Toml::file(&config_path)) + .join(Toml::file(&config_path)) //last, the configuration system wide, in /etc/odilia/config.toml - .merge(Toml::file("/etc/odilia/config.toml")); + .join(Toml::file("/etc/odilia/config.toml")); //realise the configuration and freeze it into place let config: ApplicationConfig = figment.extract()?; - tracing::debug!("configuration loaded successfully"); + tracing::debug!(?config, "configuration loaded successfully"); // Make sure applications with dynamic accessibility support do expose their AT-SPI2 interfaces. if let Err(e) = atspi_connection::set_session_accessibility(true) From 7600df904acd8f39ad55919e79dffdbcf8429a84 Mon Sep 17 00:00:00 2001 From: alberto tirla Date: Tue, 19 Mar 2024 14:02:08 +0200 Subject: [PATCH 06/19] honor configuration when setting the speech rate up to now, configuration was read, but never actually used. This begins a series of changes, perhaps across multiple fronts, to do so. This makes `state::new` send a message on the ssip channel, with the request to set the rate to a user defined value warning: if somehow the ssip task isn't initialized by the point we get there, the change will be lost in the channel, or may be picked up later than intended --- common/src/settings/speech.rs | 2 +- odilia/src/state.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/common/src/settings/speech.rs b/common/src/settings/speech.rs index e065fab7..db994677 100644 --- a/common/src/settings/speech.rs +++ b/common/src/settings/speech.rs @@ -3,7 +3,7 @@ use serde::{Deserialize, Serialize}; #[derive(Debug, Serialize, Deserialize)] #[allow(clippy::module_name_repetitions)] pub struct SpeechSettings { - pub rate: i32, + pub rate: i8, } impl Default for SpeechSettings { fn default() -> Self { diff --git a/odilia/src/state.rs b/odilia/src/state.rs index d12000b9..07cb8168 100644 --- a/odilia/src/state.rs +++ b/odilia/src/state.rs @@ -57,7 +57,7 @@ impl ScreenReaderState { let accessible_history = Mutex::new(CircularQueue::with_capacity(16)); let event_history = Mutex::new(CircularQueue::with_capacity(16)); let cache = Arc::new(Cache::new(atspi.connection().clone())); - +ssip.send(SSIPRequest::SetRate(ssip_client_async::ClientScope::Current, config.speech().rate)).await?; Ok(Self { atspi, dbus, From 9eef0b4ec403309d94d561716786f5fd2c5300c4 Mon Sep 17 00:00:00 2001 From: alberto tirla Date: Tue, 19 Mar 2024 14:18:48 +0200 Subject: [PATCH 07/19] env: attempt to parse nested configuration we use nested toml tables in our configuration, one table per what we think to be a logical section. However, because we want to add environment variables as configuration sources, we have to be able to parse nested dictionaries. In order to do that, beside just using .prefix to filter out variables which don't concern us, we must also split the string of the variable name in keys, and for that we try to use the .split method --- odilia/src/main.rs | 11 ++++------- odilia/src/state.rs | 6 +++++- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/odilia/src/main.rs b/odilia/src/main.rs index 37428c1f..e01f8705 100644 --- a/odilia/src/main.rs +++ b/odilia/src/main.rs @@ -96,14 +96,11 @@ async fn main() -> eyre::Result<()> { // Otherwise create it in XDG_CONFIG_HOME //default configuration first, because that doesn't affect the priority outlined above let figment = Figment::from(Serialized::defaults(ApplicationConfig::default())) - //environment variables - .join(Env::prefixed("ODILIA_")); + //environment variables + .join(Env::prefixed("ODILIA_").split("_")); //cli override, if applicable - let figment = if let Some(path) = args.config { - figment.join(Toml::file(path)) - } else { - figment - }; + let figment = + if let Some(path) = args.config { figment.join(Toml::file(path)) } else { figment }; //create a config.toml file in `XDG_CONFIG_HOME`, to make it possible for the user to edit the default values, if it doesn't exist already let xdg_dirs = xdg::BaseDirectories::with_prefix("odilia").expect( "unable to find the odilia config directory according to the xdg dirs specification", diff --git a/odilia/src/state.rs b/odilia/src/state.rs index 07cb8168..6ee6f2fc 100644 --- a/odilia/src/state.rs +++ b/odilia/src/state.rs @@ -57,7 +57,11 @@ impl ScreenReaderState { let accessible_history = Mutex::new(CircularQueue::with_capacity(16)); let event_history = Mutex::new(CircularQueue::with_capacity(16)); let cache = Arc::new(Cache::new(atspi.connection().clone())); -ssip.send(SSIPRequest::SetRate(ssip_client_async::ClientScope::Current, config.speech().rate)).await?; + ssip.send(SSIPRequest::SetRate( + ssip_client_async::ClientScope::Current, + config.speech().rate, + )) + .await?; Ok(Self { atspi, dbus, From be121fb38343a4f69bc1c9886bbdc8f41afbf9ee Mon Sep 17 00:00:00 2001 From: alberto tirla Date: Sun, 31 Mar 2024 13:00:30 +0300 Subject: [PATCH 08/19] refactor: move loading of configuration in its own function Cargo format --- odilia/src/main.rs | 65 ++++++++++++++++++++++++---------------------- 1 file changed, 34 insertions(+), 31 deletions(-) diff --git a/odilia/src/main.rs b/odilia/src/main.rs index e01f8705..02e8b111 100644 --- a/odilia/src/main.rs +++ b/odilia/src/main.rs @@ -13,7 +13,7 @@ mod events; mod logging; mod state; -use std::{fs, process::exit, sync::Arc, time::Duration}; +use std::{fs, path::PathBuf, process::exit, sync::Arc, time::Duration}; use crate::cli::Args; use crate::state::ScreenReaderState; @@ -91,36 +91,7 @@ async fn main() -> eyre::Result<()> { let tracker = TaskTracker::new(); tracing::debug!("Reading configuration"); - - // In order of prioritization, do environment variables, configuration via cli, then XDG_CONFIG_HOME, then /etc/odilia, - // Otherwise create it in XDG_CONFIG_HOME - //default configuration first, because that doesn't affect the priority outlined above - let figment = Figment::from(Serialized::defaults(ApplicationConfig::default())) - //environment variables - .join(Env::prefixed("ODILIA_").split("_")); - //cli override, if applicable - let figment = - if let Some(path) = args.config { figment.join(Toml::file(path)) } else { figment }; - //create a config.toml file in `XDG_CONFIG_HOME`, to make it possible for the user to edit the default values, if it doesn't exist already - let xdg_dirs = xdg::BaseDirectories::with_prefix("odilia").expect( - "unable to find the odilia config directory according to the xdg dirs specification", - ); - - let config_path = xdg_dirs - .place_config_file("config.toml") - .expect("unable to place configuration file. Maybe your system is readonly?"); - - if !config_path.exists() { - let toml = toml::to_string(&ApplicationConfig::default())?; - fs::write(&config_path, toml).expect("Unable to create default config file."); - } - //next, the xdg configuration - let figment = figment - .join(Toml::file(&config_path)) - //last, the configuration system wide, in /etc/odilia/config.toml - .join(Toml::file("/etc/odilia/config.toml")); - //realise the configuration and freeze it into place - let config: ApplicationConfig = figment.extract()?; + let config = load_configuration(args.config)?; tracing::debug!(?config, "configuration loaded successfully"); // Make sure applications with dynamic accessibility support do expose their AT-SPI2 interfaces. @@ -189,3 +160,35 @@ async fn main() -> eyre::Result<()> { .wrap_err("can not process interrupt signal"); Ok(()) } + +fn load_configuration(cli_overide: Option) -> Result { + // In order, do environment variables, a configuration file specified via cli, XDG_CONFIG_HOME, the usual location for system wide configuration(/etc/odilia/config.toml) + // If XDG_CONFIG_HOME based configuration wasn't found, create one with default values for the user to alter, for the next run of odilia + //default configuration first, because that doesn't affect the priority outlined above + let figment = Figment::from(Serialized::defaults(ApplicationConfig::default())) + //environment variables + .join(Env::prefixed("ODILIA_").split("_")); + //cli override, if applicable + let figment = + if let Some(path) = cli_overide { figment.join(Toml::file(path)) } else { figment }; + //create a config.toml file in `XDG_CONFIG_HOME`, to make it possible for the user to edit the default values, if it doesn't exist already + let xdg_dirs = xdg::BaseDirectories::with_prefix("odilia").expect( + "unable to find the odilia config directory according to the xdg dirs specification", + ); + + let config_path = xdg_dirs + .place_config_file("config.toml") + .expect("unable to place configuration file. Maybe your system is readonly?"); + + if !config_path.exists() { + let toml = toml::to_string(&ApplicationConfig::default())?; + fs::write(&config_path, toml).expect("Unable to create default config file."); + } + //next, the xdg configuration + let figment = figment + .join(Toml::file(&config_path)) + //last, the configuration system wide, in /etc/odilia/config.toml + .join(Toml::file("/etc/odilia/config.toml")); + //realise the configuration and freeze it into place + Ok(figment.extract()?) +} From fca8ce5af1aca4e1a8356ed31b34959770110b83 Mon Sep 17 00:00:00 2001 From: alberto tirla Date: Wed, 24 Apr 2024 19:33:47 +0300 Subject: [PATCH 09/19] remove environment variable configuration, as it's not working with our composite configuration keys --- odilia/src/main.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/odilia/src/main.rs b/odilia/src/main.rs index 02e8b111..183cce6b 100644 --- a/odilia/src/main.rs +++ b/odilia/src/main.rs @@ -162,12 +162,10 @@ async fn main() -> eyre::Result<()> { } fn load_configuration(cli_overide: Option) -> Result { - // In order, do environment variables, a configuration file specified via cli, XDG_CONFIG_HOME, the usual location for system wide configuration(/etc/odilia/config.toml) + // In order, do a configuration file specified via cli, XDG_CONFIG_HOME, the usual location for system wide configuration(/etc/odilia/config.toml) // If XDG_CONFIG_HOME based configuration wasn't found, create one with default values for the user to alter, for the next run of odilia //default configuration first, because that doesn't affect the priority outlined above - let figment = Figment::from(Serialized::defaults(ApplicationConfig::default())) - //environment variables - .join(Env::prefixed("ODILIA_").split("_")); + let figment = Figment::from(Serialized::defaults(ApplicationConfig::default())); //cli override, if applicable let figment = if let Some(path) = cli_overide { figment.join(Toml::file(path)) } else { figment }; From d06af707d756847dd316f2e08b867071f88edd04 Mon Sep 17 00:00:00 2001 From: alberto tirla Date: Thu, 25 Apr 2024 11:45:51 +0300 Subject: [PATCH 10/19] fix clippy warning --- odilia/src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/odilia/src/main.rs b/odilia/src/main.rs index 183cce6b..850590e7 100644 --- a/odilia/src/main.rs +++ b/odilia/src/main.rs @@ -20,7 +20,7 @@ use crate::state::ScreenReaderState; use clap::Parser; use eyre::WrapErr; use figment::{ - providers::{Env, Format, Serialized, Toml}, + providers::{Format, Serialized, Toml}, Figment, }; use futures::{future::FutureExt, StreamExt}; From 666803d07af96528c73c9ea9f8ca4f0141cc3eb0 Mon Sep 17 00:00:00 2001 From: alberto tirla Date: Thu, 25 Apr 2024 12:08:08 +0300 Subject: [PATCH 11/19] fix configuration again not joining properly --- odilia/src/main.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/odilia/src/main.rs b/odilia/src/main.rs index 850590e7..0b9b79c8 100644 --- a/odilia/src/main.rs +++ b/odilia/src/main.rs @@ -184,9 +184,9 @@ fn load_configuration(cli_overide: Option) -> Result Date: Thu, 25 Apr 2024 13:05:01 +0300 Subject: [PATCH 12/19] make logging subsystem use the logging level provided in the configuration file and make the configuration struct easier to use --- common/src/settings/mod.rs | 15 ++------------- odilia/src/logging.rs | 15 ++++++--------- odilia/src/main.rs | 8 +++++--- odilia/src/state.rs | 2 +- 4 files changed, 14 insertions(+), 26 deletions(-) diff --git a/common/src/settings/mod.rs b/common/src/settings/mod.rs index 766388a0..e85fb77e 100644 --- a/common/src/settings/mod.rs +++ b/common/src/settings/mod.rs @@ -11,18 +11,7 @@ use serde::{Deserialize, Serialize}; /// the only way this config should change is if the configuration file changes, in which case the entire view will be replaced to reflect the fact #[derive(Debug, Serialize, Deserialize, Default)] pub struct ApplicationConfig { - speech: SpeechSettings, - log: LogSettings, + pub speech: SpeechSettings, + pub log: LogSettings, } -impl ApplicationConfig { - #[must_use] - pub fn log(&self) -> &LogSettings { - &self.log - } - - #[must_use] - pub fn speech(&self) -> &SpeechSettings { - &self.speech - } -} diff --git a/odilia/src/logging.rs b/odilia/src/logging.rs index 4ba3dfce..59b6773f 100644 --- a/odilia/src/logging.rs +++ b/odilia/src/logging.rs @@ -5,24 +5,21 @@ use std::env; +use odilia_common::settings::ApplicationConfig; use tracing_error::ErrorLayer; use tracing_log::LogTracer; use tracing_subscriber::{prelude::*, EnvFilter}; use tracing_tree::HierarchicalLayer; -#[cfg(not(debug_assertions))] -const DEFAULT_LOG_FILTER: &str = "none"; -#[cfg(debug_assertions)] -const DEFAULT_LOG_FILTER: &str = "debug"; - -/// Initialise the logging stack. -pub fn init() { +/// Initialise the logging stack +/// this requires an application configuration structure, so configuration must be initialized before logging is +pub fn init(config:&ApplicationConfig) { let env_filter = match env::var("ODILIA_LOG").or_else(|_| env::var("RUST_LOG")) { Ok(s) => EnvFilter::from(s), - Err(env::VarError::NotPresent) => EnvFilter::from(DEFAULT_LOG_FILTER), + Err(env::VarError::NotPresent) => EnvFilter::from(&config.log.level), Err(e) => { eprintln!("Warning: Failed to read log filter from ODILIA_LOG or RUST_LOG: {e}"); - EnvFilter::from(DEFAULT_LOG_FILTER) + EnvFilter::from(&config.log.level) } }; let subscriber = tracing_subscriber::Registry::default() diff --git a/odilia/src/main.rs b/odilia/src/main.rs index 0b9b79c8..7cd564de 100644 --- a/odilia/src/main.rs +++ b/odilia/src/main.rs @@ -82,7 +82,6 @@ async fn sigterm_signal_watcher( #[tokio::main(flavor = "current_thread")] async fn main() -> eyre::Result<()> { let args = Args::parse(); - logging::init(); //initialize the primary token for task cancelation let token = CancellationToken::new(); @@ -90,9 +89,12 @@ async fn main() -> eyre::Result<()> { //initialize a task tracker, which will allow us to wait for all tasks to finish let tracker = TaskTracker::new(); - tracing::debug!("Reading configuration"); + //initializing configuration let config = load_configuration(args.config)?; - tracing::debug!(?config, "configuration loaded successfully"); + //initialize logging, with the provided config + logging::init(&config); + + tracing::info!(?config, "this configuration was used to prepair odilia"); // Make sure applications with dynamic accessibility support do expose their AT-SPI2 interfaces. if let Err(e) = atspi_connection::set_session_accessibility(true) diff --git a/odilia/src/state.rs b/odilia/src/state.rs index 6ee6f2fc..a8feefeb 100644 --- a/odilia/src/state.rs +++ b/odilia/src/state.rs @@ -59,7 +59,7 @@ impl ScreenReaderState { let cache = Arc::new(Cache::new(atspi.connection().clone())); ssip.send(SSIPRequest::SetRate( ssip_client_async::ClientScope::Current, - config.speech().rate, + config.speech.rate, )) .await?; Ok(Self { From 744a2cf2f200003f89ac9bd43230d272208104ae Mon Sep 17 00:00:00 2001 From: alberto tirla Date: Thu, 25 Apr 2024 19:14:41 +0300 Subject: [PATCH 13/19] allow odilia to accept a log file and use it to log information, same for the system journal and tty * specify tty in the config file for logs to be sent to your terminal * use the file option to send it to a file * use syslog for writing to the journal, for systemd equipped distros --- Cargo.lock | 30 +++++++++++++++++++++-- common/src/settings/log.rs | 27 ++++++++++++++++++-- common/src/settings/mod.rs | 4 +-- odilia/Cargo.toml | 1 + odilia/src/logging.rs | 50 ++++++++++++++++++++++++++------------ odilia/src/main.rs | 2 +- 6 files changed, 92 insertions(+), 22 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0f007f60..1b929eb0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1565,7 +1565,8 @@ dependencies = [ "toml", "tracing", "tracing-error", - "tracing-log", + "tracing-journald", + "tracing-log 0.1.4", "tracing-subscriber", "tracing-tree", "xdg", @@ -2541,6 +2542,17 @@ dependencies = [ "tracing-subscriber", ] +[[package]] +name = "tracing-journald" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ba316a74e8fc3c3896a850dba2375928a9fa171b085ecddfc7c054d39970f3fd" +dependencies = [ + "libc", + "tracing-core", + "tracing-subscriber", +] + [[package]] name = "tracing-log" version = "0.1.4" @@ -2552,6 +2564,17 @@ dependencies = [ "tracing-core", ] +[[package]] +name = "tracing-log" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ee855f1f400bd0e5c02d150ae5de3840039a3f54b025156404e34c23c03f47c3" +dependencies = [ + "log", + "once_cell", + "tracing-core", +] + [[package]] name = "tracing-subscriber" version = "0.3.18" @@ -2559,13 +2582,16 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ad0f048c97dbd9faa9b7df56362b8ebcaa52adb06b498c050d2f4e32f90a7a8b" dependencies = [ "matchers", + "nu-ansi-term", "once_cell", "parking_lot", "regex", "sharded-slab", + "smallvec", "thread_local", "tracing", "tracing-core", + "tracing-log 0.2.0", ] [[package]] @@ -2576,7 +2602,7 @@ checksum = "2ec6adcab41b1391b08a308cc6302b79f8095d1673f6947c2dc65ffb028b0b2d" dependencies = [ "nu-ansi-term", "tracing-core", - "tracing-log", + "tracing-log 0.1.4", "tracing-subscriber", ] diff --git a/common/src/settings/log.rs b/common/src/settings/log.rs index a4a1ac2f..a02a2e95 100644 --- a/common/src/settings/log.rs +++ b/common/src/settings/log.rs @@ -1,12 +1,35 @@ +use std::path::PathBuf; + use serde::{Deserialize, Serialize}; ///structure used for all the configurable options related to logging #[derive(Debug, Serialize, Deserialize)] #[allow(clippy::module_name_repetitions)] pub struct LogSettings { - level: String, + ///the logging level this session should output at + /// see the tracing documentation for more information, in the log filters section + /// typical values here include info, warn, debug and trace + /// however, one can also include specific modules for which logging should be shown at a different warning level + pub level: String, + ///the place where odilia should output its logs + /// the values possible include tty, file and syslog + pub logger: LoggingKind, } impl Default for LogSettings { fn default() -> Self { - Self { level: "info".to_owned() } + Self { level: "info".to_owned(), logger: LoggingKind::File("/var/log/odilia.log".into()) } } } + +///the place where odilia should output its logs +#[derive(Serialize, Deserialize, Debug)] +pub enum LoggingKind { + ///a file where the log messages should be written + /// the path can be both absolute and relative to the current working directory + /// warning: the path must be accessible permission wise from the user where odilia was launched + File(PathBuf), + ///logs are being sent to the terminal directly + Tty, + ///the logs are sent to systemd-journald, as long as the target architecture supports it + /// if that's not the case, this option does nothing + Syslog, +} diff --git a/common/src/settings/mod.rs b/common/src/settings/mod.rs index e85fb77e..e0a40191 100644 --- a/common/src/settings/mod.rs +++ b/common/src/settings/mod.rs @@ -1,5 +1,5 @@ -mod log; -mod speech; +pub mod log; +pub mod speech; use log::LogSettings; use speech::SpeechSettings; diff --git a/odilia/Cargo.toml b/odilia/Cargo.toml index 1e2c0881..794d7a4e 100644 --- a/odilia/Cargo.toml +++ b/odilia/Cargo.toml @@ -58,6 +58,7 @@ clap = { version = "4.5.1", features = ["derive"] } tokio-util.workspace=true toml = "0.8.11" figment = { version = "0.10.14", features = ["env", "toml"] } +tracing-journald = "0.3.0" [dev-dependencies] lazy_static = "1.4.0" diff --git a/odilia/src/logging.rs b/odilia/src/logging.rs index 59b6773f..278b7387 100644 --- a/odilia/src/logging.rs +++ b/odilia/src/logging.rs @@ -5,7 +5,8 @@ use std::env; -use odilia_common::settings::ApplicationConfig; +use eyre::Context; +use odilia_common::settings::{log::LoggingKind, ApplicationConfig}; use tracing_error::ErrorLayer; use tracing_log::LogTracer; use tracing_subscriber::{prelude::*, EnvFilter}; @@ -13,29 +14,48 @@ use tracing_tree::HierarchicalLayer; /// Initialise the logging stack /// this requires an application configuration structure, so configuration must be initialized before logging is -pub fn init(config:&ApplicationConfig) { - let env_filter = match env::var("ODILIA_LOG").or_else(|_| env::var("RUST_LOG")) { - Ok(s) => EnvFilter::from(s), - Err(env::VarError::NotPresent) => EnvFilter::from(&config.log.level), - Err(e) => { - eprintln!("Warning: Failed to read log filter from ODILIA_LOG or RUST_LOG: {e}"); - EnvFilter::from(&config.log.level) +pub fn init(config: &ApplicationConfig) -> eyre::Result<()> { + let env_filter = + match env::var("APP_LOG").or_else(|_| env::var("RUST_LOG")) { + Ok(s) => EnvFilter::from(s), + Err(env::VarError::NotPresent) => EnvFilter::from(&config.log.level), + Err(e) => { + eprintln!("Warning: Failed to read log filter from APP_LOG or RUST_LOG: {e}"); + EnvFilter::from(&config.log.level) + } + }; + //this requires boxing because the types returned by this match block would be incompatible otherwise, since we return different layers depending on what we get from the configuration. It is possible to do it otherwise, hopefully, but for now this and a forced dereference at the end would do + let output_layer = match &config.log.logger { + LoggingKind::File(path) => { + let file = std::fs::File::options() + .create_new(true) + .write(true) + .open(path) + .with_context(|| { + format!("creating log file '{}'", path.display()) + })?; + let fmt = + tracing_subscriber::fmt::layer().with_ansi(false).with_writer(file); + fmt.boxed() } + LoggingKind::Tty => tracing_subscriber::fmt::layer() + .with_ansi(true) + .with_target(true) + .boxed(), + LoggingKind::Syslog => tracing_journald::layer()?.boxed(), }; let subscriber = tracing_subscriber::Registry::default() .with(env_filter) + .with(output_layer) .with(ErrorLayer::default()) .with(HierarchicalLayer::new(4) - .with_ansi(true) .with_bracketed_fields(true) .with_targets(true) .with_deferred_spans(true) .with_span_retrace(true) .with_indent_lines(true)); - if let Err(e) = tracing::subscriber::set_global_default(subscriber) { - eprintln!("Warning: Failed to set log handler: {e}"); - } - if let Err(e) = LogTracer::init() { - tracing::warn!(error = %e, "Failed to install log facade"); - } + tracing::subscriber::set_global_default(subscriber) + .wrap_err("unable to init default logging layer")?; + LogTracer::init().wrap_err("unable to init tracing log layer")?; + Ok(()) } diff --git a/odilia/src/main.rs b/odilia/src/main.rs index 7cd564de..427b568f 100644 --- a/odilia/src/main.rs +++ b/odilia/src/main.rs @@ -92,7 +92,7 @@ async fn main() -> eyre::Result<()> { //initializing configuration let config = load_configuration(args.config)?; //initialize logging, with the provided config - logging::init(&config); + logging::init(&config)?; tracing::info!(?config, "this configuration was used to prepair odilia"); From 1f5d5aa8b41a3be6d5a5c4eae28dd81d2c3c04dd Mon Sep 17 00:00:00 2001 From: alberto tirla Date: Thu, 25 Apr 2024 21:19:19 +0300 Subject: [PATCH 14/19] add pitch to the configuration --- common/src/settings/speech.rs | 3 ++- odilia/src/state.rs | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/common/src/settings/speech.rs b/common/src/settings/speech.rs index db994677..bd6db059 100644 --- a/common/src/settings/speech.rs +++ b/common/src/settings/speech.rs @@ -4,9 +4,10 @@ use serde::{Deserialize, Serialize}; #[allow(clippy::module_name_repetitions)] pub struct SpeechSettings { pub rate: i8, + pub pitch: i8, } impl Default for SpeechSettings { fn default() -> Self { - Self { rate: 50 } + Self { rate: 50, pitch: 0} } } diff --git a/odilia/src/state.rs b/odilia/src/state.rs index a8feefeb..63e37598 100644 --- a/odilia/src/state.rs +++ b/odilia/src/state.rs @@ -57,6 +57,7 @@ impl ScreenReaderState { let accessible_history = Mutex::new(CircularQueue::with_capacity(16)); let event_history = Mutex::new(CircularQueue::with_capacity(16)); let cache = Arc::new(Cache::new(atspi.connection().clone())); + ssip.send(SSIPRequest::SetPitch(ssip_client_async::ClientScope::Current, config.speech.pitch)).await?s; ssip.send(SSIPRequest::SetRate( ssip_client_async::ClientScope::Current, config.speech.rate, From 981091abfa66d27895d7062236e7ee3b82e8d34f Mon Sep 17 00:00:00 2001 From: alberto tirla Date: Thu, 25 Apr 2024 21:42:01 +0300 Subject: [PATCH 15/19] add volume configuration --- common/src/settings/speech.rs | 3 ++- odilia/src/state.rs | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/common/src/settings/speech.rs b/common/src/settings/speech.rs index bd6db059..85f605d2 100644 --- a/common/src/settings/speech.rs +++ b/common/src/settings/speech.rs @@ -5,9 +5,10 @@ use serde::{Deserialize, Serialize}; pub struct SpeechSettings { pub rate: i8, pub pitch: i8, + pub volume: i8, } impl Default for SpeechSettings { fn default() -> Self { - Self { rate: 50, pitch: 0} + Self { rate: 50, pitch: 0, volume: 100} } } diff --git a/odilia/src/state.rs b/odilia/src/state.rs index 63e37598..4cd940a4 100644 --- a/odilia/src/state.rs +++ b/odilia/src/state.rs @@ -57,7 +57,9 @@ impl ScreenReaderState { let accessible_history = Mutex::new(CircularQueue::with_capacity(16)); let event_history = Mutex::new(CircularQueue::with_capacity(16)); let cache = Arc::new(Cache::new(atspi.connection().clone())); - ssip.send(SSIPRequest::SetPitch(ssip_client_async::ClientScope::Current, config.speech.pitch)).await?s; + ssip.send(SSIPRequest::SetPitch(ssip_client_async::ClientScope::Current, config.speech.pitch)).await?; + ssip.send(SSIPRequest::SetVolume(ssip_client_async::ClientScope::Current, config.speech.volume)).await?; + ssip.send(SSIPRequest::SetRate( ssip_client_async::ClientScope::Current, config.speech.rate, From 22f3ad707f0543a149ed61a7b580eb420d25ae82 Mon Sep 17 00:00:00 2001 From: alberto tirla Date: Thu, 25 Apr 2024 21:56:24 +0300 Subject: [PATCH 16/19] make output module configurable and run the formatter a bit --- common/src/settings/speech.rs | 3 ++- odilia/src/state.rs | 18 +++++++++++++++--- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/common/src/settings/speech.rs b/common/src/settings/speech.rs index 85f605d2..a20a0d55 100644 --- a/common/src/settings/speech.rs +++ b/common/src/settings/speech.rs @@ -6,9 +6,10 @@ pub struct SpeechSettings { pub rate: i8, pub pitch: i8, pub volume: i8, + pub module: String, } impl Default for SpeechSettings { fn default() -> Self { - Self { rate: 50, pitch: 0, volume: 100} + Self { rate: 50, pitch: 0, volume: 100, module: "espeak-ng".into()} } } diff --git a/odilia/src/state.rs b/odilia/src/state.rs index 4cd940a4..5ae6bb8c 100644 --- a/odilia/src/state.rs +++ b/odilia/src/state.rs @@ -57,9 +57,21 @@ impl ScreenReaderState { let accessible_history = Mutex::new(CircularQueue::with_capacity(16)); let event_history = Mutex::new(CircularQueue::with_capacity(16)); let cache = Arc::new(Cache::new(atspi.connection().clone())); - ssip.send(SSIPRequest::SetPitch(ssip_client_async::ClientScope::Current, config.speech.pitch)).await?; - ssip.send(SSIPRequest::SetVolume(ssip_client_async::ClientScope::Current, config.speech.volume)).await?; - + ssip.send(SSIPRequest::SetPitch( + ssip_client_async::ClientScope::Current, + config.speech.pitch, + )) + .await?; + ssip.send(SSIPRequest::SetVolume( + ssip_client_async::ClientScope::Current, + config.speech.volume, + )) + .await?; + ssip.send(SSIPRequest::SetOutputModule( + ssip_client_async::ClientScope::Current, + config.speech.module.clone(), + )) + .await?; ssip.send(SSIPRequest::SetRate( ssip_client_async::ClientScope::Current, config.speech.rate, From 068c80e4b6b892d627ba868c702e51c50a68c147 Mon Sep 17 00:00:00 2001 From: alberto tirla Date: Thu, 25 Apr 2024 22:49:28 +0300 Subject: [PATCH 17/19] add language and voice to the configuration --- common/src/settings/speech.rs | 4 +++- odilia/src/state.rs | 10 ++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/common/src/settings/speech.rs b/common/src/settings/speech.rs index a20a0d55..5ba3dd49 100644 --- a/common/src/settings/speech.rs +++ b/common/src/settings/speech.rs @@ -7,9 +7,11 @@ pub struct SpeechSettings { pub pitch: i8, pub volume: i8, pub module: String, + pub language: String, + pub person: String, } impl Default for SpeechSettings { fn default() -> Self { - Self { rate: 50, pitch: 0, volume: 100, module: "espeak-ng".into()} + Self { rate: 50, pitch: 0, volume: 100, module: "espeak-ng".into(), language: "en-US".into(), person: "English (America)+Max".into()} } } diff --git a/odilia/src/state.rs b/odilia/src/state.rs index 5ae6bb8c..cc4f352e 100644 --- a/odilia/src/state.rs +++ b/odilia/src/state.rs @@ -72,6 +72,16 @@ impl ScreenReaderState { config.speech.module.clone(), )) .await?; + ssip.send(SSIPRequest::SetLanguage( + ssip_client_async::ClientScope::Current, + config.speech.language.clone(), + )) + .await?; + ssip.send(SSIPRequest::SetSynthesisVoice( + ssip_client_async::ClientScope::Current, + config.speech.person.clone(), + )) + .await?; ssip.send(SSIPRequest::SetRate( ssip_client_async::ClientScope::Current, config.speech.rate, From 4aca3a39abedf0d1748898842e3ed502e079f9bf Mon Sep 17 00:00:00 2001 From: alberto tirla Date: Thu, 25 Apr 2024 23:45:07 +0300 Subject: [PATCH 18/19] make punctuation reporting configurable --- common/src/settings/speech.rs | 11 ++++++++++- odilia/src/state.rs | 12 ++++++++++-- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/common/src/settings/speech.rs b/common/src/settings/speech.rs index 5ba3dd49..34aaae7b 100644 --- a/common/src/settings/speech.rs +++ b/common/src/settings/speech.rs @@ -9,9 +9,18 @@ pub struct SpeechSettings { pub module: String, pub language: String, pub person: String, + pub punctuation: PunctuationSpellingMode, } impl Default for SpeechSettings { fn default() -> Self { - Self { rate: 50, pitch: 0, volume: 100, module: "espeak-ng".into(), language: "en-US".into(), person: "English (America)+Max".into()} + Self { rate: 50, pitch: 0, volume: 100, module: "espeak-ng".into(), language: "en-US".into(), person: "English (America)+Max".into(), punctuation: PunctuationSpellingMode::Some} } } + +#[derive(Clone, Copy, Debug, Serialize, Deserialize)] +pub enum PunctuationSpellingMode{ + Some, + Most, + None, + All, +} \ No newline at end of file diff --git a/odilia/src/state.rs b/odilia/src/state.rs index cc4f352e..83fa09d9 100644 --- a/odilia/src/state.rs +++ b/odilia/src/state.rs @@ -2,7 +2,7 @@ use std::sync::atomic::AtomicI32; use circular_queue::CircularQueue; use eyre::WrapErr; -use ssip_client_async::{MessageScope, Priority, Request as SSIPRequest}; +use ssip_client_async::{MessageScope, Priority, PunctuationMode, Request as SSIPRequest}; use tokio::sync::{mpsc::Sender, Mutex}; use tracing::{debug, Instrument}; use zbus::{fdo::DBusProxy, names::UniqueName, zvariant::ObjectPath, MatchRule, MessageType}; @@ -16,7 +16,7 @@ use atspi_connection::AccessibilityConnection; use atspi_proxies::{accessible::AccessibleProxy, cache::CacheProxy}; use odilia_cache::{AccessiblePrimitive, Cache, CacheItem}; use odilia_common::{ - errors::CacheError, modes::ScreenReaderMode, settings::ApplicationConfig, + errors::CacheError, modes::ScreenReaderMode, settings::{speech::PunctuationSpellingMode, ApplicationConfig}, types::TextSelectionArea, Result as OdiliaResult, }; use std::sync::Arc; @@ -82,6 +82,14 @@ impl ScreenReaderState { config.speech.person.clone(), )) .await?; + //doing it this way for now. It could have been done with a From impl, but I don't want to make ssip_client_async a dependency of odilia_common, so this conversion is done directly inside state, especially since this enum isn't supposed to grow any further, in complexity or variants + let punctuation_mode=match config.speech.punctuation{ + PunctuationSpellingMode::Some => PunctuationMode::Some, + PunctuationSpellingMode::Most => PunctuationMode::Most, + PunctuationSpellingMode::None => PunctuationMode::None, + PunctuationSpellingMode::All => PunctuationMode::All, + }; + ssip.send(SSIPRequest::SetPunctuationMode(ssip_client_async::ClientScope::Current, punctuation_mode)).await?; ssip.send(SSIPRequest::SetRate( ssip_client_async::ClientScope::Current, config.speech.rate, From 949a72d109f9b8ec0c83a76f7a1f9458b6ac3f0c Mon Sep 17 00:00:00 2001 From: alberto tirla Date: Thu, 25 Apr 2024 23:45:43 +0300 Subject: [PATCH 19/19] fix formatting --- common/src/settings/log.rs | 5 ++++- common/src/settings/mod.rs | 1 - common/src/settings/speech.rs | 14 +++++++++++--- odilia/src/state.rs | 15 +++++++++++---- 4 files changed, 26 insertions(+), 9 deletions(-) diff --git a/common/src/settings/log.rs b/common/src/settings/log.rs index a02a2e95..d687f976 100644 --- a/common/src/settings/log.rs +++ b/common/src/settings/log.rs @@ -16,7 +16,10 @@ pub struct LogSettings { } impl Default for LogSettings { fn default() -> Self { - Self { level: "info".to_owned(), logger: LoggingKind::File("/var/log/odilia.log".into()) } + Self { + level: "info".to_owned(), + logger: LoggingKind::File("/var/log/odilia.log".into()), + } } } diff --git a/common/src/settings/mod.rs b/common/src/settings/mod.rs index e0a40191..2830feb0 100644 --- a/common/src/settings/mod.rs +++ b/common/src/settings/mod.rs @@ -14,4 +14,3 @@ pub struct ApplicationConfig { pub speech: SpeechSettings, pub log: LogSettings, } - diff --git a/common/src/settings/speech.rs b/common/src/settings/speech.rs index 34aaae7b..bf08d722 100644 --- a/common/src/settings/speech.rs +++ b/common/src/settings/speech.rs @@ -13,14 +13,22 @@ pub struct SpeechSettings { } impl Default for SpeechSettings { fn default() -> Self { - Self { rate: 50, pitch: 0, volume: 100, module: "espeak-ng".into(), language: "en-US".into(), person: "English (America)+Max".into(), punctuation: PunctuationSpellingMode::Some} + Self { + rate: 50, + pitch: 0, + volume: 100, + module: "espeak-ng".into(), + language: "en-US".into(), + person: "English (America)+Max".into(), + punctuation: PunctuationSpellingMode::Some, + } } } #[derive(Clone, Copy, Debug, Serialize, Deserialize)] -pub enum PunctuationSpellingMode{ +pub enum PunctuationSpellingMode { Some, Most, None, All, -} \ No newline at end of file +} diff --git a/odilia/src/state.rs b/odilia/src/state.rs index 83fa09d9..0fdff840 100644 --- a/odilia/src/state.rs +++ b/odilia/src/state.rs @@ -16,8 +16,11 @@ use atspi_connection::AccessibilityConnection; use atspi_proxies::{accessible::AccessibleProxy, cache::CacheProxy}; use odilia_cache::{AccessiblePrimitive, Cache, CacheItem}; use odilia_common::{ - errors::CacheError, modes::ScreenReaderMode, settings::{speech::PunctuationSpellingMode, ApplicationConfig}, - types::TextSelectionArea, Result as OdiliaResult, + errors::CacheError, + modes::ScreenReaderMode, + settings::{speech::PunctuationSpellingMode, ApplicationConfig}, + types::TextSelectionArea, + Result as OdiliaResult, }; use std::sync::Arc; @@ -83,13 +86,17 @@ impl ScreenReaderState { )) .await?; //doing it this way for now. It could have been done with a From impl, but I don't want to make ssip_client_async a dependency of odilia_common, so this conversion is done directly inside state, especially since this enum isn't supposed to grow any further, in complexity or variants - let punctuation_mode=match config.speech.punctuation{ + let punctuation_mode = match config.speech.punctuation { PunctuationSpellingMode::Some => PunctuationMode::Some, PunctuationSpellingMode::Most => PunctuationMode::Most, PunctuationSpellingMode::None => PunctuationMode::None, PunctuationSpellingMode::All => PunctuationMode::All, }; - ssip.send(SSIPRequest::SetPunctuationMode(ssip_client_async::ClientScope::Current, punctuation_mode)).await?; + ssip.send(SSIPRequest::SetPunctuationMode( + ssip_client_async::ClientScope::Current, + punctuation_mode, + )) + .await?; ssip.send(SSIPRequest::SetRate( ssip_client_async::ClientScope::Current, config.speech.rate,