Issue13: System checksums (#35)

* Implement what I know of System checksums

* Add more tests (12 fail)

* Fix mising options in PedalFunction enum  (3 tests fail)

* Fix deserialization of escaped strings (tests pass)

* Validate that system only values aren't used for LiveSets

* Add validate test (1 test fails)

* Remove test cases from generated files

src/json/serialize_chars_as_string.rs

@@ -1,10 +1,12 @@
+use std::borrow::Cow;
+
 use schemars::{schema::{StringValidation, SchemaObject, InstanceType}, JsonSchema};
 use serde::{de, Serialize, Deserialize};
 
 pub fn deserialize<'de, D, const N: usize>(deserializer: D) -> Result<[char; N], D::Error> 
 where D: serde::Deserializer<'de>
 {
-    let s = <&str>::deserialize(deserializer)?;
+    let s = <Cow<'de, str>>::deserialize(deserializer)?; // Cow is required because if the string contains escaped chars (eg. //), you'll get an owned string instead of a borrowed string, but Cow can handle either
     let chars: Vec<char> = s.chars().collect();
     if chars.len() != N {
         Err(de::Error::custom(format!("Expected {} chars, but got {}", N, chars.len())))

src/json/validation.rs

@@ -7,6 +7,7 @@ use std::hash::Hash;
 use std::cmp::Eq;
 
 use crate::roland::layers::LogicalLayer;
+use crate::roland::types::enums::{ButtonFunction, PedalFunction};
 use crate::roland::types::notes::PianoKey;
 
 pub fn valid_chars<const N: usize>(chars: &[char; N]) -> Result<(), ValidationError> {
@@ -148,3 +149,23 @@ pub fn valid_velocity_range<T: LayerRanges>(layer: &T) -> Result<(), ValidationE
         Ok(())
     }
 }
+
+pub fn not_system_only_button_function(value: &ButtonFunction) -> Result<(), ValidationError> {
+    if value.is_system_only() {
+        let mut e = ValidationError::new("System only ButtonFunction used for LiveSet");
+        e.add_param(Cow::from("button_function"), value);
+        Err(e)
+    } else {
+        Ok(())
+    }
+}
+
+pub fn not_system_only_pedal_function(value: &PedalFunction) -> Result<(), ValidationError> {
+    if value.is_system_only() {
+        let mut e = ValidationError::new("System only PedalFunction used for LiveSet");
+        e.add_param(Cow::from("pedal_function"), value);
+        Err(e)
+    } else {
+        Ok(())
+    }
+}

src/roland/live_set/common.rs

@@ -6,7 +6,7 @@ use validator::Validate;
 
 use crate::bytes::{Bytes, BytesError, Bits, BitStream};
 use crate::json::{Json, StructuredJson, serialize_chars_as_string, StructuredJsonError};
-use crate::json::validation::{valid_chars, contains_all_keys};
+use crate::json::validation::{valid_chars, contains_all_keys, not_system_only_button_function, not_system_only_pedal_function};
 use crate::roland::types::StateMap;
 use crate::roland::types::enums::{Layer, SliderSelect, KeyOffPosition, KeyTouchVelocity, KeyTouchCurveType, VoiceReserve, HarmonicBar, MidiChannel, ButtonFunction, PedalFunction, SliderFunction, SoundFocusType};
 use crate::roland::types::numeric::OffsetU8;
@@ -28,15 +28,19 @@ pub struct Common {
     #[validate(range(min = 10, max = 500))]
     live_set_tempo: u16,
     #[validate]
+    #[validate(custom = "not_system_only_pedal_function")]
     pub fc1_assign: PedalFunction, // 0-144
     #[validate]
+    #[validate(custom = "not_system_only_pedal_function")]
     pub fc2_assign: PedalFunction, // 0-144
     sound_focus_switch: bool,
     #[validate]
     sound_focus_type: SoundFocusType,
     #[validate(range(max = 127))]
     sound_focus_value: u8,
+    #[validate(custom = "not_system_only_button_function")]
     s1_assign: ButtonFunction, // 0-17
+    #[validate(custom = "not_system_only_button_function")]
     s2_assign: ButtonFunction, // 0-17
     s1_state: bool,
     s2_state: bool,

src/roland/live_set/mod.rs

@@ -14,6 +14,7 @@ use self::reverb::Reverb;
 use self::song_rhythm::SongRhythm;
 
 use super::layers::{LogicalLayer, ToneWheelLayer, EPianoLayer, InternalLayer, ExternalLayer, ToneLayer, PianoLayer};
+use super::sum_to_zero;
 
 mod common;
 pub mod chorus;
@@ -205,10 +206,6 @@ impl Json for LiveSet {
     }
 }
 
-fn sum_to_zero(sum: u16) -> u8 {
-    (u8::MAX - sum.to_be_bytes()[1]).wrapping_add(1)
-}
-
 impl Warnings for LiveSet {
     fn warnings(&self) -> Vec<String> {
         let mut warnings = Vec::new();

src/roland/mod.rs

@@ -7,3 +7,7 @@ pub mod types;
 
 #[cfg(test)]
 mod tests;
+
+fn sum_to_zero(sum: u16) -> u8 {
+    (u8::MAX - sum.to_be_bytes()[1]).wrapping_add(1)
+}

src/roland/system/mod.rs

@@ -4,6 +4,7 @@ use validator::Validate;
 use crate::bytes::{Bytes, BytesError, Bits, BitStream};
 use crate::json::{Json, StructuredJson, StructuredJsonError, serialize_chars_as_string};
 use crate::json::validation::valid_chars;
+use super::sum_to_zero;
 use self::common::Common;
 use self::compressor::Compressor;
 use self::favorites::Favorites;
@@ -22,27 +23,28 @@ pub struct System {
     padding1: Bits<16>, // 2 bytes padding
     #[validate]
     pub common: Common, // 10 bytes
-    unsure2: Bits<16>, // 2 bytes Common checksum?
+    checksum1: SystemCheckSum, // 2 bytes checksum
     #[serde(skip_serializing_if="Bits::is_unit", default="Bits::<16>::unit")]
     padding2: Bits<16>, // 2 bytes padding
     //#[validate]
     compressor: Compressor, // 14 bytes
-    unsure3: Bits<16>, // 2 bytes Compressor checksum?
+    checksum2: SystemCheckSum, // 2 bytes checksum
     #[serde(skip_serializing_if="Bits::is_unit", default="Bits::<16>::unit")]
     padding3: Bits<16>, // 2 bytes padding
     //#[validate]
     v_link: VLink, // 4 bytes
-    unsure4: Bits<16>, // 2 bytes VLink checksum?
+    checksum3: SystemCheckSum, // 2 bytes checksum
+    // 2 bytes VLink checksum?
     #[serde(skip_serializing_if="Bits::is_unit", default="Bits::<16>::unit")]
     padding4: Bits<16>, // 2 bytes padding
     #[validate]
     favorites: Favorites, // 76 bytes
-    unsure5: Bits<16>, // 2 bytes Favorites checksum?
+    checksum4: SystemCheckSum, // 2 bytes checksum
     #[serde(skip_serializing_if="Bits::is_unit", default="Bits::<16>::unit")]
     padding5: Bits<16>, // 2 bytes padding
     //#[validate]
     switch_assign: SwitchAssign, // 20 bytes
-    unsure6: Bits<16>, // 2 bytes SwitchAssign checksum?
+    checksum5: SystemCheckSum, // 2 bytes checksum
     #[serde(deserialize_with = "serialize_chars_as_string::deserialize")]
     #[serde(serialize_with = "serialize_chars_as_string::serialize")]
     #[schemars(with = "serialize_chars_as_string::StringSchema::<16>")]
@@ -53,41 +55,41 @@ pub struct System {
 impl Bytes<160> for System {
     fn from_bytes(bytes: Box<[u8; Self::BYTE_SIZE]>) -> Result<Self, BytesError> {
         BitStream::read_fixed(bytes, |data| {
-            let padding1 = data.get_bits();
+            let padding1: Bits<16> = data.get_bits();
             let common = Common::from_bytes(data.get_bytes())?;
-            let unsure2 = data.get_bits();
+            let checksum1 = SystemCheckSum::read(data)?;
             let padding2 = data.get_bits();
             let compressor = Compressor::from_bytes(data.get_bytes())?;
-            let unsure3 = data.get_bits();
+            let checksum2 = SystemCheckSum::read(data)?;
             let padding3 = data.get_bits();
             let v_link = VLink::from_bytes(data.get_bytes())?;
-            let unsure4 = data.get_bits();
+            let checksum3 = SystemCheckSum::read(data)?;
             let padding4 = data.get_bits();
             let favorites = Favorites::from_bytes(data.get_bytes())?;
-            let unsure5 = data.get_bits();
+            let checksum4 = SystemCheckSum::read(data)?;
             let padding5 = data.get_bits();
             let switch_assign = SwitchAssign::from_bytes(data.get_bytes())?;
-            let unsure6 = data.get_bits();
+            let checksum5 = SystemCheckSum::read(data)?;
             let mut hardware_version = [char::default(); 16];
             for i in 0..hardware_version.len() {
                 hardware_version[i] = data.get_char::<8>()?;
             }
             Ok(Self {
                 padding1,
                 common,
-                unsure2,
+                checksum1,
                 padding2,
                 compressor,
-                unsure3,
+                checksum2,
                 padding3,
                 v_link,
-                unsure4,
+                checksum3,
                 padding4,
                 favorites,
-                unsure5,
+                checksum4,
                 padding5,
                 switch_assign,
-                unsure6,
+                checksum5,
                 hardware_version
             })
         })
@@ -97,19 +99,19 @@ impl Bytes<160> for System {
         BitStream::write_fixed(|bs| {
             bs.set_bits(&self.padding1);
             bs.set_bytes(self.common.to_bytes()?);
-            bs.set_bits(&self.unsure2);
+            self.checksum1.write(bs);
             bs.set_bits(&self.padding2);
             bs.set_bytes(self.compressor.to_bytes()?);
-            bs.set_bits(&self.unsure3);
+            self.checksum2.write(bs);
             bs.set_bits(&self.padding3);
             bs.set_bytes(self.v_link.to_bytes()?);
-            bs.set_bits(&self.unsure4);
+            self.checksum3.write(bs);
             bs.set_bits(&self.padding4);
             bs.set_bytes(self.favorites.to_bytes()?);
-            bs.set_bits(&self.unsure5);
+            self.checksum4.write(bs);
             bs.set_bits(&self.padding5);
             bs.set_bytes(self.switch_assign.to_bytes()?);
-            bs.set_bits(&self.unsure6);
+            self.checksum5.write(bs);
             for ch in self.hardware_version {
                 bs.set_char::<8>(ch)?;
             }
@@ -135,3 +137,45 @@ impl Json for System {
         serde_json::from_str(&json)
     }
 }
+
+/*
+System section checksums are a bit screwy.
+
+For each System section (eg. Common, VLink, etc.) there is a 2 byte checksum. On the keyboard, you can only "Write" (save) one System section at a time.
+The checksum for the section is set when that section is "Written" (saved), BUT its value depends on the bytes of all System bytes before it, *including other sections*.
+This means that a checksum can be (and more often than not is) out of date. This will happen whenever a System section is "Written" (saved) without re-writing all sections following it.
+For this reason, the checksum must actually be stored (at least partially).
+
+What I know about the check sum mathematically:
+- the sum of the padding bytes, section bytes and both checksum bytes is zero
+- how the 2 checksum bytes is chosen exactly is a mystery (because there are many combinations of bytes which would cause the sum to be zero)
+- the way the 2 checksums bytes are chosen depends on System section preceeding the current one (except presumably for the first section)
+- it appears (likely, not confirmed) that changing the preceeding System sections affects the choice of the 2nd byte of checksum
+*/
+#[derive(Serialize, Deserialize, JsonSchema)]
+struct SystemCheckSum(u8); // store the 2nd byte only, as the 1st can be calculated as the checksum
+
+impl SystemCheckSum {
+    fn read(data: &mut BitStream) -> Result<Self, BytesError> {
+        let mut sum = data.sum_previous_bytes();
+        let first = data.get_full_u8();
+        let second = data.get_full_u8();
+        sum = sum.wrapping_add(second as u16);
+        let expected_first = sum_to_zero(sum);
+        if first == expected_first {
+            Ok(Self(second))
+        } else {
+            Err(BytesError::IncorrectCheckSum {
+                expected: vec![expected_first],
+                found: vec![first]
+            })
+        }
+    }
+
+    fn write(&self, data: &mut BitStream) {
+        let sum = data.sum_previous_bytes().wrapping_add(self.0 as u16);
+        let first = sum_to_zero(sum);
+        data.set_full_u8(first);
+        data.set_full_u8(self.0);
+    }
+}