From e7905a5b8ff31103dc1260229845fbc6d6762ee1 Mon Sep 17 00:00:00 2001 From: Orne Brocaar Date: Tue, 14 Jan 2025 09:19:41 +0000 Subject: [PATCH] Refactor device-profile abp fields. This this puts the ABP parameters into a single JSON(B) field, to reduce the amount of device-profile fields that currently exist. The same work will be done for Class-B/C and Relay parameters. Once completed, this means we can drop the diesel '64-column-tables' feature, which will reduce compile time. --- Makefile | 1 - .../down.sql | 18 ++++++ .../up.sql | 17 ++++++ .../down.sql | 16 ++++++ .../up.sql | 16 ++++++ chirpstack/src/api/device_profile.rs | 37 +++++++++---- chirpstack/src/maccommand/reset.rs | 12 ++-- chirpstack/src/storage/device_profile.rs | 26 ++++----- .../src/storage/fields/device_profile.rs | 55 +++++++++++++++++++ chirpstack/src/storage/fields/mod.rs | 2 + chirpstack/src/storage/schema_postgres.rs | 5 +- chirpstack/src/storage/schema_sqlite.rs | 5 +- shell.nix | 1 + 13 files changed, 169 insertions(+), 42 deletions(-) create mode 100644 chirpstack/migrations_postgres/2025-01-13-152218_refactor_device_profile_fields/down.sql create mode 100644 chirpstack/migrations_postgres/2025-01-13-152218_refactor_device_profile_fields/up.sql create mode 100644 chirpstack/migrations_sqlite/2025-01-13-163304_refactor_device_profile_fields/down.sql create mode 100644 chirpstack/migrations_sqlite/2025-01-13-163304_refactor_device_profile_fields/up.sql create mode 100644 chirpstack/src/storage/fields/device_profile.rs diff --git a/Makefile b/Makefile index 5cc78b08..5406bc95 100644 --- a/Makefile +++ b/Makefile @@ -8,7 +8,6 @@ dist: # Install dev dependencies dev-dependencies: cargo install cross --version 0.2.5 - cargo install diesel_cli --version 2.2.1 --no-default-features --features postgres,sqlite cargo install cargo-deb --version 1.43.1 cargo install cargo-generate-rpm --version 0.12.1 diff --git a/chirpstack/migrations_postgres/2025-01-13-152218_refactor_device_profile_fields/down.sql b/chirpstack/migrations_postgres/2025-01-13-152218_refactor_device_profile_fields/down.sql new file mode 100644 index 00000000..f8856929 --- /dev/null +++ b/chirpstack/migrations_postgres/2025-01-13-152218_refactor_device_profile_fields/down.sql @@ -0,0 +1,18 @@ +alter table device_profile + add column abp_rx1_delay smallint not null default 0, + add column abp_rx1_dr_offset smallint not null default 0, + add column abp_rx2_dr smallint not null default 0, + add column abp_rx2_freq bigint not null default 0; + +update device_profile + set + abp_rx1_delay = (abp_params->'rx1_delay')::smallint, + abp_rx1_dr_offset = (abp_params->'rx1_dr_offset')::smallint, + abp_rx2_dr = (abp_params->'rx2_dr')::smallint, + abp_rx2_freq = (abp_params->'rx2_freq')::bigint + where + abp_params is not null; + +alter table device_profile + drop column abp_params; + diff --git a/chirpstack/migrations_postgres/2025-01-13-152218_refactor_device_profile_fields/up.sql b/chirpstack/migrations_postgres/2025-01-13-152218_refactor_device_profile_fields/up.sql new file mode 100644 index 00000000..c5d2ae1b --- /dev/null +++ b/chirpstack/migrations_postgres/2025-01-13-152218_refactor_device_profile_fields/up.sql @@ -0,0 +1,17 @@ +alter table device_profile + add column abp_params jsonb null; + +update device_profile + set abp_params = json_build_object( + 'rx1_delay', abp_rx1_delay, + 'rx1_dr_offset', abp_rx1_dr_offset, + 'rx2_dr', abp_rx2_dr, + 'rx2_freq', abp_rx2_freq) + where supports_otaa = false; + +alter table device_profile + drop column abp_rx1_delay, + drop column abp_rx1_dr_offset, + drop column abp_rx2_dr, + drop column abp_rx2_freq; + diff --git a/chirpstack/migrations_sqlite/2025-01-13-163304_refactor_device_profile_fields/down.sql b/chirpstack/migrations_sqlite/2025-01-13-163304_refactor_device_profile_fields/down.sql new file mode 100644 index 00000000..2d4ed06c --- /dev/null +++ b/chirpstack/migrations_sqlite/2025-01-13-163304_refactor_device_profile_fields/down.sql @@ -0,0 +1,16 @@ +alter table device_profile add column abp_rx1_delay smallint not null default 0; +alter table device_profile add column abp_rx1_dr_offset smallint not null default 0; +alter table device_profile add column abp_rx2_dr smallint not null default 0; +alter table device_profile add column abp_rx2_freq bigint not null default 0; + +update device_profile + set + abp_rx1_delay = abp_params->'rx1_delay', + abp_rx1_dr_offset = abp_params->'rx1_dr_offset', + abp_rx2_dr = abp_params->'rx2_dr', + abp_rx2_freq = abp_params->'rx2_freq' + where + abp_params is not null; + +alter table device_profile drop column abp_params; + diff --git a/chirpstack/migrations_sqlite/2025-01-13-163304_refactor_device_profile_fields/up.sql b/chirpstack/migrations_sqlite/2025-01-13-163304_refactor_device_profile_fields/up.sql new file mode 100644 index 00000000..b3aceafc --- /dev/null +++ b/chirpstack/migrations_sqlite/2025-01-13-163304_refactor_device_profile_fields/up.sql @@ -0,0 +1,16 @@ +alter table device_profile + add column abp_params text null; + +update device_profile + set abp_params = json_object( + 'rx1_delay', abp_rx1_delay, + 'rx1_dr_offset', abp_rx1_dr_offset, + 'rx2_dr', abp_rx2_dr, + 'rx2_freq', abp_rx2_freq) + where supports_otaa = false; + +alter table device_profile drop column abp_rx1_delay; +alter table device_profile drop column abp_rx1_dr_offset; +alter table device_profile drop column abp_rx2_dr; +alter table device_profile drop column abp_rx2_freq; + diff --git a/chirpstack/src/api/device_profile.rs b/chirpstack/src/api/device_profile.rs index 7c7c709c..42f802f5 100644 --- a/chirpstack/src/api/device_profile.rs +++ b/chirpstack/src/api/device_profile.rs @@ -65,10 +65,6 @@ impl DeviceProfileService for DeviceProfile { class_b_ping_slot_dr: req_dp.class_b_ping_slot_dr as i16, class_b_ping_slot_freq: req_dp.class_b_ping_slot_freq as i64, class_c_timeout: req_dp.class_c_timeout as i32, - abp_rx1_delay: req_dp.abp_rx1_delay as i16, - abp_rx1_dr_offset: req_dp.abp_rx1_dr_offset as i16, - abp_rx2_dr: req_dp.abp_rx2_dr as i16, - abp_rx2_freq: req_dp.abp_rx2_freq as i64, tags: fields::KeyValue::new(req_dp.tags.clone()), measurements: fields::Measurements::new( req_dp @@ -114,6 +110,16 @@ impl DeviceProfileService for DeviceProfile { relay_overall_limit_bucket_size: req_dp.relay_overall_limit_bucket_size as i16, allow_roaming: req_dp.allow_roaming, rx1_delay: req_dp.rx1_delay as i16, + abp_params: if req_dp.supports_otaa { + None + } else { + Some(fields::AbpParams { + rx1_delay: req_dp.abp_rx1_delay as u8, + rx1_dr_offset: req_dp.abp_rx1_dr_offset as u8, + rx2_dr: req_dp.abp_rx2_dr as u8, + rx2_freq: req_dp.abp_rx2_freq as u32, + }) + }, ..Default::default() }; @@ -145,6 +151,7 @@ impl DeviceProfileService for DeviceProfile { .await?; let dp = device_profile::get(&dp_id).await.map_err(|e| e.status())?; + let abp_params = dp.abp_params.clone().unwrap_or_default(); let mut resp = Response::new(api::GetDeviceProfileResponse { device_profile: Some(api::DeviceProfile { @@ -169,10 +176,10 @@ impl DeviceProfileService for DeviceProfile { class_b_ping_slot_dr: dp.class_b_ping_slot_dr as u32, class_b_ping_slot_freq: dp.class_b_ping_slot_freq as u32, class_c_timeout: dp.class_c_timeout as u32, - abp_rx1_delay: dp.abp_rx1_delay as u32, - abp_rx1_dr_offset: dp.abp_rx1_dr_offset as u32, - abp_rx2_dr: dp.abp_rx2_dr as u32, - abp_rx2_freq: dp.abp_rx2_freq as u32, + abp_rx1_delay: abp_params.rx1_delay as u32, + abp_rx1_dr_offset: abp_params.rx1_dr_offset as u32, + abp_rx2_dr: abp_params.rx2_dr as u32, + abp_rx2_freq: abp_params.rx2_freq as u32, tags: dp.tags.into_hashmap(), measurements: dp .measurements @@ -267,10 +274,6 @@ impl DeviceProfileService for DeviceProfile { class_b_ping_slot_dr: req_dp.class_b_ping_slot_dr as i16, class_b_ping_slot_freq: req_dp.class_b_ping_slot_freq as i64, class_c_timeout: req_dp.class_c_timeout as i32, - abp_rx1_delay: req_dp.abp_rx1_delay as i16, - abp_rx1_dr_offset: req_dp.abp_rx1_dr_offset as i16, - abp_rx2_dr: req_dp.abp_rx2_dr as i16, - abp_rx2_freq: req_dp.abp_rx2_freq as i64, tags: fields::KeyValue::new(req_dp.tags.clone()), measurements: fields::Measurements::new( req_dp @@ -316,6 +319,16 @@ impl DeviceProfileService for DeviceProfile { relay_overall_limit_bucket_size: req_dp.relay_overall_limit_bucket_size as i16, allow_roaming: req_dp.allow_roaming, rx1_delay: req_dp.rx1_delay as i16, + abp_params: if req_dp.supports_otaa { + None + } else { + Some(fields::AbpParams { + rx1_delay: req_dp.abp_rx1_delay as u8, + rx1_dr_offset: req_dp.abp_rx1_dr_offset as u8, + rx2_dr: req_dp.abp_rx2_dr as u8, + rx2_freq: req_dp.abp_rx2_freq as u32, + }) + }, ..Default::default() }) .await diff --git a/chirpstack/src/maccommand/reset.rs b/chirpstack/src/maccommand/reset.rs index e7fcd556..c2f62783 100644 --- a/chirpstack/src/maccommand/reset.rs +++ b/chirpstack/src/maccommand/reset.rs @@ -1,7 +1,7 @@ use anyhow::Result; use tracing::info; -use crate::storage::{device, device_profile}; +use crate::storage::{device, device_profile, fields}; const SERV_LORAWAN_VERSION: lrwn::Version = lrwn::Version::LoRaWAN1_1; @@ -70,13 +70,15 @@ pub mod test { }; let dp = device_profile::DeviceProfile { supports_otaa: false, - abp_rx1_delay: 1, - abp_rx1_dr_offset: 0, - abp_rx2_dr: 0, - abp_rx2_freq: 868300000, class_b_ping_slot_dr: 2, class_b_ping_slot_freq: 868100000, class_b_ping_slot_nb_k: 1, + abp_params: Some(fields::AbpParams { + rx1_delay: 1, + rx1_dr_offset: 0, + rx2_dr: 0, + rx2_freq: 868300000, + }), ..Default::default() }; diff --git a/chirpstack/src/storage/device_profile.rs b/chirpstack/src/storage/device_profile.rs index dc91d3ba..4c5ddb0c 100644 --- a/chirpstack/src/storage/device_profile.rs +++ b/chirpstack/src/storage/device_profile.rs @@ -39,10 +39,6 @@ pub struct DeviceProfile { pub class_b_ping_slot_dr: i16, pub class_b_ping_slot_freq: i64, pub class_c_timeout: i32, - pub abp_rx1_delay: i16, - pub abp_rx1_dr_offset: i16, - pub abp_rx2_dr: i16, - pub abp_rx2_freq: i64, pub tags: fields::KeyValue, pub payload_codec_script: String, pub flush_queue_on_activate: bool, @@ -74,6 +70,7 @@ pub struct DeviceProfile { pub relay_overall_limit_bucket_size: i16, pub allow_roaming: bool, pub rx1_delay: i16, + pub abp_params: Option, } impl DeviceProfile { @@ -118,10 +115,6 @@ impl Default for DeviceProfile { class_b_ping_slot_dr: 0, class_b_ping_slot_freq: 0, class_c_timeout: 0, - abp_rx1_delay: 0, - abp_rx1_dr_offset: 0, - abp_rx2_dr: 0, - abp_rx2_freq: 0, tags: fields::KeyValue::new(HashMap::new()), measurements: fields::Measurements::new(HashMap::new()), auto_detect_measurements: false, @@ -150,6 +143,7 @@ impl Default for DeviceProfile { relay_overall_limit_bucket_size: 0, allow_roaming: false, rx1_delay: 0, + abp_params: None, } } } @@ -174,11 +168,14 @@ impl DeviceProfile { ds.min_supported_tx_power_index = 0; ds.max_supported_tx_power_index = 0; ds.extra_uplink_channels = HashMap::new(); - ds.rx1_delay = self.abp_rx1_delay as u32; - ds.rx1_dr_offset = self.abp_rx1_dr_offset as u32; - ds.rx2_dr = self.abp_rx2_dr as u32; - ds.rx2_frequency = self.abp_rx2_freq as u32; ds.enabled_uplink_channel_indices = Vec::new(); + + if let Some(abp_params) = &self.abp_params { + ds.rx1_delay = abp_params.rx1_delay as u32; + ds.rx1_dr_offset = abp_params.rx1_dr_offset as u32; + ds.rx2_dr = abp_params.rx2_dr as u32; + ds.rx2_frequency = abp_params.rx2_freq as u32; + } } } } @@ -249,10 +246,6 @@ pub async fn update(dp: DeviceProfile) -> Result { device_profile::class_b_ping_slot_dr.eq(&dp.class_b_ping_slot_dr), device_profile::class_b_ping_slot_freq.eq(&dp.class_b_ping_slot_freq), device_profile::class_c_timeout.eq(&dp.class_c_timeout), - device_profile::abp_rx1_delay.eq(&dp.abp_rx1_delay), - device_profile::abp_rx1_dr_offset.eq(&dp.abp_rx1_dr_offset), - device_profile::abp_rx2_dr.eq(&dp.abp_rx2_dr), - device_profile::abp_rx2_freq.eq(&dp.abp_rx2_freq), device_profile::tags.eq(&dp.tags), device_profile::measurements.eq(&dp.measurements), device_profile::auto_detect_measurements.eq(&dp.auto_detect_measurements), @@ -287,6 +280,7 @@ pub async fn update(dp: DeviceProfile) -> Result { device_profile::relay_overall_limit_bucket_size.eq(&dp.relay_overall_limit_bucket_size), device_profile::allow_roaming.eq(&dp.allow_roaming), device_profile::rx1_delay.eq(&dp.rx1_delay), + device_profile::abp_params.eq(&dp.abp_params), )) .get_result(&mut get_async_db_conn().await?) .await diff --git a/chirpstack/src/storage/fields/device_profile.rs b/chirpstack/src/storage/fields/device_profile.rs new file mode 100644 index 00000000..cc707468 --- /dev/null +++ b/chirpstack/src/storage/fields/device_profile.rs @@ -0,0 +1,55 @@ +use diesel::backend::Backend; +use diesel::{deserialize, serialize}; +#[cfg(feature = "postgres")] +use diesel::{pg::Pg, sql_types::Jsonb}; +#[cfg(feature = "sqlite")] +use diesel::{sql_types::Text, sqlite::Sqlite}; +use serde::{Deserialize, Serialize}; + +#[derive( + Default, Debug, Clone, PartialEq, Eq, Deserialize, Serialize, AsExpression, FromSqlRow, +)] +#[cfg_attr(feature = "postgres", diesel(sql_type = Jsonb))] +#[cfg_attr(feature = "sqlite", diesel(sql_type = Text))] +pub struct AbpParams { + pub rx1_delay: u8, + pub rx1_dr_offset: u8, + pub rx2_dr: u8, + pub rx2_freq: u32, +} + +#[cfg(feature = "postgres")] +impl deserialize::FromSql for AbpParams { + fn from_sql(value: ::RawValue<'_>) -> deserialize::Result { + let value = >::from_sql(value)?; + Ok(serde_json::from_value(value)?) + } +} + +#[cfg(feature = "postgres")] +impl serialize::ToSql for AbpParams { + fn to_sql<'b>(&'b self, out: &mut serialize::Output<'b, '_, Pg>) -> serialize::Result { + let value = serde_json::to_value(&self)?; + >::to_sql(&value, &mut out.reborrow()) + } +} + +#[cfg(feature = "sqlite")] +impl deserialize::FromSql for AbpParams +where + *const str: deserialize::FromSql, +{ + fn from_sql(value: ::RawValue<'_>) -> deserialize::Result { + let s = + <*const str as deserialize::FromSql>::from_sql(value)?; + Ok(serde_json::from_str(unsafe { &*s })?) + } +} + +#[cfg(feature = "sqlite")] +impl serialize::ToSql for AbpParams { + fn to_sql<'b>(&'b self, out: &mut serialize::Output<'b, '_, Sqlite>) -> serialize::Result { + out.set_value(serde_json::to_string(&self)?); + Ok(serialize::IsNull::No) + } +} diff --git a/chirpstack/src/storage/fields/mod.rs b/chirpstack/src/storage/fields/mod.rs index 75e80363..ba89ba47 100644 --- a/chirpstack/src/storage/fields/mod.rs +++ b/chirpstack/src/storage/fields/mod.rs @@ -1,5 +1,6 @@ mod big_decimal; mod dev_nonces; +mod device_profile; mod device_session; mod key_value; mod measurements; @@ -8,6 +9,7 @@ mod uuid; pub use big_decimal::BigDecimal; pub use dev_nonces::DevNonces; +pub use device_profile::AbpParams; pub use device_session::DeviceSession; pub use key_value::KeyValue; pub use measurements::*; diff --git a/chirpstack/src/storage/schema_postgres.rs b/chirpstack/src/storage/schema_postgres.rs index 1eeae44a..e1b11cd7 100644 --- a/chirpstack/src/storage/schema_postgres.rs +++ b/chirpstack/src/storage/schema_postgres.rs @@ -108,10 +108,6 @@ diesel::table! { class_b_ping_slot_dr -> Int2, class_b_ping_slot_freq -> Int8, class_c_timeout -> Int4, - abp_rx1_delay -> Int2, - abp_rx1_dr_offset -> Int2, - abp_rx2_dr -> Int2, - abp_rx2_freq -> Int8, tags -> Jsonb, payload_codec_script -> Text, flush_queue_on_activate -> Bool, @@ -144,6 +140,7 @@ diesel::table! { relay_overall_limit_bucket_size -> Int2, allow_roaming -> Bool, rx1_delay -> Int2, + abp_params -> Nullable, } } diff --git a/chirpstack/src/storage/schema_sqlite.rs b/chirpstack/src/storage/schema_sqlite.rs index a7d380b7..088b295b 100644 --- a/chirpstack/src/storage/schema_sqlite.rs +++ b/chirpstack/src/storage/schema_sqlite.rs @@ -97,10 +97,6 @@ diesel::table! { class_b_ping_slot_dr -> SmallInt, class_b_ping_slot_freq -> BigInt, class_c_timeout -> Integer, - abp_rx1_delay -> SmallInt, - abp_rx1_dr_offset -> SmallInt, - abp_rx2_dr -> SmallInt, - abp_rx2_freq -> BigInt, tags -> Text, payload_codec_script -> Text, flush_queue_on_activate -> Bool, @@ -132,6 +128,7 @@ diesel::table! { relay_overall_limit_bucket_size -> SmallInt, allow_roaming -> Bool, rx1_delay -> SmallInt, + abp_params -> Nullable, } } diff --git a/shell.nix b/shell.nix index 6c7b39a6..ba8c2823 100644 --- a/shell.nix +++ b/shell.nix @@ -20,6 +20,7 @@ pkgs.mkShell { pkgs.protoc-gen-go-grpc pkgs.openssl pkgs.sqlite + pkgs.diesel-cli ]; LIBCLANG_PATH = "${pkgs.llvmPackages.libclang.lib}/lib"; BINDGEN_EXTRA_CLANG_ARGS = "-I${pkgs.llvmPackages.libclang.lib}/lib/clang/${pkgs.llvmPackages.libclang.version}/include";