From c1b9a0f4ffad4312a179e1210136a1c31f108489 Mon Sep 17 00:00:00 2001
From: Robin Mueller <muellerr@irs.uni-stuttgart.de>
Date: Mon, 23 Jan 2023 16:18:06 +0100
Subject: [PATCH] some improvements, docs and smaller stuff

---
 CHANGELOG.md                          |  4 ++++
 mission/devices/SyrlinksHkHandler.cpp | 14 ++++++++------
 mission/devices/SyrlinksHkHandler.h   | 22 ++++++++++++++--------
 3 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1a407d96..9dc21472 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -18,6 +18,7 @@ list yields a list of all related PRs for each release.
   PR: https://egit.irs.uni-stuttgart.de/eive/eive-obsw/pulls/340.
 - Basic TCS Subsystem component.
   PR: https://egit.irs.uni-stuttgart.de/eive/eive-obsw/pulls/319
+
 ## Changed
 
 - Moved some PDEC/PTME configuration to `common/config/eive/definitions.h`
@@ -42,6 +43,9 @@ list yields a list of all related PRs for each release.
   previous range setting was wrong. Also fixed a small error properly set internal state
   on shut-down.
   PR: https://egit.irs.uni-stuttgart.de/eive/eive-obsw/pulls/342
+- Syrlinks Handler: Read RX frequency shift as 24 bit signed number now. Also include
+  validity handling for datasets.
+  PR: https://egit.irs.uni-stuttgart.de/eive/eive-obsw/pulls/350
 
 # [v1.19.0] 2023-01-10
 
diff --git a/mission/devices/SyrlinksHkHandler.cpp b/mission/devices/SyrlinksHkHandler.cpp
index a004a479..52cf862d 100644
--- a/mission/devices/SyrlinksHkHandler.cpp
+++ b/mission/devices/SyrlinksHkHandler.cpp
@@ -34,8 +34,8 @@ void SyrlinksHkHandler::doStartUp() {
 }
 
 void SyrlinksHkHandler::doShutDown() {
-	setMode(_MODE_POWER_DOWN);
-	temperatureSet.setValidity(false, true);
+  setMode(_MODE_POWER_DOWN);
+  temperatureSet.setValidity(false, true);
 }
 
 ReturnValue_t SyrlinksHkHandler::buildNormalDeviceCommand(DeviceCommandId_t* id) {
@@ -506,19 +506,21 @@ void SyrlinksHkHandler::parseRxStatusRegistersReply(const uint8_t* packet) {
   rxDataset.rxStatus = convertHexStringToUint8(reinterpret_cast<const char*>(packet + offset));
   offset += 2;
   rxDataset.rxSensitivity =
-      convertHexStringToRaw<uint32_t>(reinterpret_cast<const char*>(packet + offset), 6);
+      convertHexStringTo32bit<uint32_t>(reinterpret_cast<const char*>(packet + offset), 6);
   offset += 6;
   rxDataset.rxFrequencyShift =
-      convertHexStringToRaw<int32_t>(reinterpret_cast<const char*>(packet + offset), 6);
+      convertHexStringTo32bit<int32_t>(reinterpret_cast<const char*>(packet + offset), 6);
   offset += 6;
   rxDataset.rxIqPower = convertHexStringToUint16(reinterpret_cast<const char*>(packet + offset));
   offset += 4;
   rxDataset.rxAgcValue = convertHexStringToUint16(reinterpret_cast<const char*>(packet + offset));
   offset += 4;
   offset += 2;  // reserved register
-  rxDataset.rxDemodEb = convertHexStringToRaw<uint32_t>(reinterpret_cast<const char*>(packet + offset), 6);
+  rxDataset.rxDemodEb =
+      convertHexStringTo32bit<uint32_t>(reinterpret_cast<const char*>(packet + offset), 6);
   offset += 6;
-  rxDataset.rxDemodN0 = convertHexStringToRaw<uint32_t>(reinterpret_cast<const char*>(packet + offset), 6);
+  rxDataset.rxDemodN0 =
+      convertHexStringTo32bit<uint32_t>(reinterpret_cast<const char*>(packet + offset), 6);
   offset += 6;
   rxDataset.rxDataRate = convertHexStringToUint8(reinterpret_cast<const char*>(packet + offset));
 
diff --git a/mission/devices/SyrlinksHkHandler.h b/mission/devices/SyrlinksHkHandler.h
index 225d5f43..f19610f0 100644
--- a/mission/devices/SyrlinksHkHandler.h
+++ b/mission/devices/SyrlinksHkHandler.h
@@ -147,16 +147,16 @@ class SyrlinksHkHandler : public DeviceHandlerBase {
   uint16_t convertHexStringToUint16(const char* fourChars);
 
   /**
-   * @brief   Function converts a hex number represented by 6 or 8 characters to an uint32_t.
+   * @brief   Function converts a hex number represented by 6 or 8 characters to an uint32_t or
+   * int32_t, depending on the template parameter.
    *
    * @param characters    Pointer to the hex characters array.
    * @param numberOfChars Number of characters representing the hex value. Must be 6 or 8.
    *
-   * @return  The uint32_t value.
+   * @return  The value.
    */
-  // uint32_t convertHexStringToUint32(const char* characters, uint8_t numberOfChars);
   template <typename T>
-  T convertHexStringToRaw(const char* characters, uint8_t numberOfChars);
+  T convertHexStringTo32bit(const char* characters, uint8_t numberOfChars);
   /**
    * @brief   This function parses the status reply
    * @param status    Pointer to the status information.
@@ -216,15 +216,21 @@ class SyrlinksHkHandler : public DeviceHandlerBase {
 };
 
 template <typename T>
-T SyrlinksHkHandler::convertHexStringToRaw(const char* characters,
-                                                     uint8_t numberOfChars) {
+T SyrlinksHkHandler::convertHexStringTo32bit(const char* characters, uint8_t numberOfChars) {
+  if (sizeof(T) < 4) {
+    sif::error << "SyrlinksHkHandler::convertHexStringToRaw: Only works for 32-bit conversion"
+               << std::endl;
+  }
   T value = 0;
 
   switch (numberOfChars) {
     case 6:
+      // The bitshift trickery required is necessary when creating an int32_t from a
+      // 24 bit signed value.
       value = ((convertHexStringToUint8(characters) << 24) |
-              (convertHexStringToUint8(characters + 2) << 16) |
-              (convertHexStringToUint8(characters + 4) << 8)) >> 8;
+               (convertHexStringToUint8(characters + 2) << 16) |
+               (convertHexStringToUint8(characters + 4) << 8)) >>
+              8;
       return value;
     case 8:
       value = convertHexStringToUint8(characters) << 24 |