Op Divider and bitutility updates

- Added unittests for `PeriodicOperationDivider` and the `bitutil` helpers
- Some API changes: Removed redundant bit part, because these functions are already in a namespace
- Some bugfixes for `PeriodicOperationDivider`

hal/src/fsfw_hal/devicehandlers/MgmRM3100Handler.cpp

@@ -186,7 +186,7 @@ ReturnValue_t MgmRM3100Handler::interpretDeviceReply(DeviceCommandId_t id, const
         uint8_t cmmValue = packet[1];
         // We clear the seventh bit in any case
         // because this one is zero sometimes for some reason
-        bitutil::bitClear(&cmmValue, 6);
+        bitutil::clear(&cmmValue, 6);
         if(cmmValue == cmmRegValue and internalState == InternalState::READ_CMM) {
             commandExecuted = true;
         }

src/fsfw/datapoollocal/LocalPoolDataSetBase.cpp

@@ -110,7 +110,7 @@ ReturnValue_t LocalPoolDataSetBase::serializeWithValidityBuffer(uint8_t **buffer
     for (uint16_t count = 0; count < fillCount; count++) {
         if(registeredVariables[count]->isValid()) {
             /* Set bit at correct position */
-            bitutil::bitSet(validityPtr + validBufferIndex, validBufferIndexBit);
+            bitutil::set(validityPtr + validBufferIndex, validBufferIndexBit);
         }
         if(validBufferIndexBit == 7) {
             validBufferIndex ++;
@@ -156,8 +156,8 @@ ReturnValue_t LocalPoolDataSetBase::deSerializeWithValidityBuffer(
     uint8_t validBufferIndexBit = 0;
     for (uint16_t count = 0; count < fillCount; count++) {
         // set validity buffer here.
-        bool nextVarValid = bitutil::bitGet(*buffer +
+        bool nextVarValid = false;
-                validBufferIndex, validBufferIndexBit);
+        bitutil::get(*buffer + validBufferIndex, validBufferIndexBit, nextVarValid);
         registeredVariables[count]->setValid(nextVarValid);
 
         if(validBufferIndexBit == 7) {

src/fsfw/globalfunctions/PeriodicOperationDivider.cpp

@@ -2,43 +2,40 @@
 
 
 PeriodicOperationDivider::PeriodicOperationDivider(uint32_t divider,
-		bool resetAutomatically): resetAutomatically(resetAutomatically),
+        bool resetAutomatically): resetAutomatically(resetAutomatically),
-		counter(divider), divider(divider) {
+        divider(divider) {
 }
 
 bool PeriodicOperationDivider::checkAndIncrement() {
-	bool opNecessary = check();
+    bool opNecessary = check();
-	if(opNecessary) {
+    if(opNecessary and resetAutomatically) {
-		if(resetAutomatically) {
+        resetCounter();
-			counter = 0;
+    }
-		}
+    else {
-		return opNecessary;
+        counter++;
-	}
+    }
-	counter ++;
+    return opNecessary;
-	return opNecessary;
 }
 
 bool PeriodicOperationDivider::check() {
-	if(counter >= divider) {
+    if(counter >= divider) {
-		return true;
+        return true;
-	}
+    }
-	return false;
+    return false;
 }
 
-
-
 void PeriodicOperationDivider::resetCounter() {
-	counter = 0;
+    counter = 1;
 }
 
 void PeriodicOperationDivider::setDivider(uint32_t newDivider) {
-	divider = newDivider;
+    divider = newDivider;
 }
 
 uint32_t PeriodicOperationDivider::getCounter() const {
-	return counter;
+    return counter;
 }
 
 uint32_t PeriodicOperationDivider::getDivider() const {
-	return divider;
+    return divider;
 }

src/fsfw/globalfunctions/PeriodicOperationDivider.h

@@ -13,51 +13,50 @@
  */
 class PeriodicOperationDivider {
 public:
-	/**
+    /**
-	 * Initialize with the desired divider and specify whether the internal
+     * Initialize with the desired divider and specify whether the internal
-	 * counter will be reset automatically.
+     * counter will be reset automatically.
-	 * @param divider
+     * @param divider Value of 0 or 1 will cause #check and #checkAndIncrement to always return
-	 * @param resetAutomatically
+     * true
-	 */
+     * @param resetAutomatically
-	PeriodicOperationDivider(uint32_t divider, bool resetAutomatically = true);
+     */
+    PeriodicOperationDivider(uint32_t divider, bool resetAutomatically = true);
 
+    /**
+     * Check whether operation is necessary. If an operation is necessary and the class has been
+     * configured to be reset automatically, the counter will be reset to 1 automatically
+     *
+     * @return
+     * -@c true if the counter is larger or equal to the divider
+     * -@c false otherwise
+     */
+    bool checkAndIncrement();
 
-	/**
+    /**
-	 * Check whether operation is necessary.
+     * Checks whether an operation is necessary. This function will not increment the counter.
-	 * If an operation is necessary and the class has been
+     * @return
-	 * configured to be reset automatically, the counter will be reset.
+     * -@c true if the counter is larger or equal to the divider
-	 *
+     * -@c false otherwise
-	 * @return
+     */
-	 * -@c true if the counter is larger or equal to the divider
+    bool check();
-	 * -@c false otherwise
-	 */
-	bool checkAndIncrement();
 
-	/**
+    /**
-	 * Checks whether an operation is necessary.
+     * Can be used to reset the counter to 1 manually
-	 * This function will not increment the counter!
+     */
-	 * @return
+    void resetCounter();
-	 * -@c true if the counter is larger or equal to the divider
+    uint32_t getCounter() const;
-	 * -@c false otherwise
-	 */
-	bool check();
 
-	/**
+    /**
-	 * Can be used to reset the counter to 0 manually.
+     * Can be used to set a new divider value.
-	 */
+     * @param newDivider
-	void resetCounter();
+     */
-	uint32_t getCounter() const;
+    void setDivider(uint32_t newDivider);
+    uint32_t getDivider() const;
 
-	/**
-	 * Can be used to set a new divider value.
-	 * @param newDivider
-	 */
-	void setDivider(uint32_t newDivider);
-	uint32_t getDivider() const;
 private:
-	bool resetAutomatically = true;
+    bool resetAutomatically = true;
-	uint32_t counter = 0;
+    uint32_t counter = 1;
-	uint32_t divider = 0;
+    uint32_t divider = 0;
 };
 
 

src/fsfw/globalfunctions/bitutility.cpp

@@ -1,6 +1,6 @@
 #include "fsfw/globalfunctions/bitutility.h"
 
-void bitutil::bitSet(uint8_t *byte, uint8_t position) {
+void bitutil::set(uint8_t *byte, uint8_t position) {
     if(position > 7) {
         return;
     }
@@ -8,7 +8,7 @@ void bitutil::bitSet(uint8_t *byte, uint8_t position) {
     *byte |= 1 << shiftNumber;
 }
 
-void bitutil::bitToggle(uint8_t *byte, uint8_t position) {
+void bitutil::toggle(uint8_t *byte, uint8_t position) {
     if(position > 7) {
         return;
     }
@@ -16,7 +16,7 @@ void bitutil::bitToggle(uint8_t *byte, uint8_t position) {
     *byte ^= 1 << shiftNumber;
 }
 
-void bitutil::bitClear(uint8_t *byte, uint8_t position) {
+void bitutil::clear(uint8_t *byte, uint8_t position) {
     if(position > 7) {
         return;
     }
@@ -24,10 +24,11 @@ void bitutil::bitClear(uint8_t *byte, uint8_t position) {
     *byte &= ~(1 << shiftNumber);
 }
 
-bool bitutil::bitGet(const uint8_t *byte, uint8_t position) {
+bool bitutil::get(const uint8_t *byte, uint8_t position, bool& bit) {
     if(position > 7) {
         return false;
     }
     uint8_t shiftNumber = position + (7 - 2 * position);
-    return *byte & (1 << shiftNumber);
+    bit = *byte & (1 << shiftNumber);
+    return true;
 }

src/fsfw/globalfunctions/bitutility.h

@@ -5,13 +5,36 @@
 
 namespace bitutil {
 
-/* Helper functions for manipulating the individual bits of a byte.
+// Helper functions for manipulating the individual bits of a byte.
-Position refers to n-th bit of a byte, going from 0 (most significant bit) to
+// Position refers to n-th bit of a byte, going from 0 (most significant bit) to
-7 (least significant bit) */
+// 7 (least significant bit)
-void bitSet(uint8_t* byte, uint8_t position);
+
-void bitToggle(uint8_t* byte, uint8_t position);
+/**
-void bitClear(uint8_t* byte, uint8_t position);
+ * @brief   Set the bit in a given byte
-bool bitGet(const uint8_t* byte, uint8_t position);
+ * @param byte
+ * @param position
+ */
+void set(uint8_t* byte, uint8_t position);
+/**
+ * @brief   Toggle the bit in a given byte
+ * @param byte
+ * @param position
+ */
+void toggle(uint8_t* byte, uint8_t position);
+/**
+ * @brief   Clear the bit in a given byte
+ * @param byte
+ * @param position
+ */
+void clear(uint8_t* byte, uint8_t position);
+/**
+ * @brief   Get the bit in a given byte
+ * @param byte
+ * @param position
+ * @param If the input is valid, this will be set to true if the bit is set and false otherwise.
+ * @return False if position is invalid, True otherwise
+ */
+bool get(const uint8_t* byte, uint8_t position, bool& bit);
 
 }
 

tests/src/fsfw_tests/unit/datapoollocal/DataSetTest.cpp

@@ -171,14 +171,19 @@ TEST_CASE("DataSetTest" , "[DataSetTest]") {
             /* We can do it like this because the buffer only has one byte for
             less than 8 variables */
             uint8_t* validityByte = buffer + sizeof(buffer) - 1;
-            CHECK(bitutil::bitGet(validityByte, 0) == true);
+            bool bitSet = false;
-            CHECK(bitutil::bitGet(validityByte, 1) == false);
+            bitutil::get(validityByte, 0, bitSet);
-            CHECK(bitutil::bitGet(validityByte, 2) == true);
+
+            CHECK(bitSet == true);
+            bitutil::get(validityByte, 1, bitSet);
+            CHECK(bitSet == false);
+            bitutil::get(validityByte, 2, bitSet);
+            CHECK(bitSet == true);
 
             /* Now we manipulate the validity buffer for the deserialization */
-            bitutil::bitClear(validityByte, 0);
+            bitutil::clear(validityByte, 0);
-            bitutil::bitSet(validityByte, 1);
+            bitutil::set(validityByte, 1);
-            bitutil::bitClear(validityByte, 2);
+            bitutil::clear(validityByte, 2);
             /* Zero out everything except validity buffer */
             std::memset(buffer, 0, sizeof(buffer) - 1);
             sizeToDeserialize = maxSize;
@@ -239,8 +244,11 @@ TEST_CASE("DataSetTest" , "[DataSetTest]") {
         std::memcpy(validityBuffer.data(), buffer + 9 + sizeof(uint16_t) * 3, 2);
         /* The first 9 variables should be valid */
         CHECK(validityBuffer[0] == 0xff);
-        CHECK(bitutil::bitGet(validityBuffer.data() + 1, 0) == true);
+        bool bitSet = false;
-        CHECK(bitutil::bitGet(validityBuffer.data() + 1, 1) == false);
+        bitutil::get(validityBuffer.data() + 1, 0, bitSet);
+        CHECK(bitSet == true);
+        bitutil::get(validityBuffer.data() + 1, 1, bitSet);
+        CHECK(bitSet == false);
 
         /* Now we invert the validity */
         validityBuffer[0] = 0;

tests/src/fsfw_tests/unit/globalfunctions/CMakeLists.txt

@@ -1,3 +1,5 @@
 target_sources(${FSFW_TEST_TGT} PRIVATE
     testDleEncoder.cpp
+    testOpDivider.cpp
+    testBitutil.cpp
 )

tests/src/fsfw_tests/unit/globalfunctions/testBitutil.cpp

@@ -0,0 +1,64 @@
+#include "fsfw/globalfunctions/bitutility.h"
+#include <catch2/catch_test_macros.hpp>
+
+TEST_CASE("Bitutility" , "[Bitutility]") {
+    uint8_t dummyByte = 0;
+    bool bitSet = false;
+    for(uint8_t pos = 0; pos < 8; pos++) {
+        bitutil::set(&dummyByte, pos);
+        REQUIRE(dummyByte == (1 << (7 - pos)));
+        bitutil::get(&dummyByte, pos, bitSet);
+        REQUIRE(bitSet == 1);
+        dummyByte = 0;
+    }
+
+    dummyByte = 0xff;
+    for(uint8_t pos = 0; pos < 8; pos++) {
+        bitutil::get(&dummyByte, pos, bitSet);
+        REQUIRE(bitSet == 1);
+        bitutil::clear(&dummyByte, pos);
+        bitutil::get(&dummyByte, pos, bitSet);
+        REQUIRE(bitSet == 0);
+        dummyByte = 0xff;
+    }
+
+    dummyByte = 0xf0;
+    for(uint8_t pos = 0; pos < 8; pos++) {
+        if(pos < 4) {
+            bitutil::get(&dummyByte, pos, bitSet);
+            REQUIRE(bitSet == 1);
+            bitutil::toggle(&dummyByte, pos);
+            bitutil::get(&dummyByte, pos, bitSet);
+            REQUIRE(bitSet == 0);
+        }
+        else {
+            bitutil::get(&dummyByte, pos, bitSet);
+            REQUIRE(bitSet == false);
+            bitutil::toggle(&dummyByte, pos);
+            bitutil::get(&dummyByte, pos, bitSet);
+            REQUIRE(bitSet == true);
+        }
+    }
+    REQUIRE(dummyByte == 0x0f);
+
+    dummyByte = 0;
+    bitutil::set(&dummyByte, 8);
+    REQUIRE(dummyByte == 0);
+    bitutil::set(&dummyByte, -1);
+    REQUIRE(dummyByte == 0);
+    dummyByte = 0xff;
+    bitutil::clear(&dummyByte, 8);
+    REQUIRE(dummyByte == 0xff);
+    bitutil::clear(&dummyByte, -1);
+    REQUIRE(dummyByte == 0xff);
+    dummyByte = 0x00;
+    bitutil::toggle(&dummyByte, 8);
+    REQUIRE(dummyByte == 0x00);
+    bitutil::toggle(&dummyByte, -1);
+    REQUIRE(dummyByte == 0x00);
+
+    REQUIRE(bitutil::get(&dummyByte, 8, bitSet) == false);
+}
+
+
+

tests/src/fsfw_tests/unit/globalfunctions/testOpDivider.cpp

@@ -0,0 +1,64 @@
+#include "fsfw/globalfunctions/PeriodicOperationDivider.h"
+#include <catch2/catch_test_macros.hpp>
+
+TEST_CASE("OpDivider" , "[OpDivider]") {
+    auto opDivider = PeriodicOperationDivider(1);
+    REQUIRE(opDivider.getDivider() == 1);
+    REQUIRE(opDivider.getCounter() == 1);
+    REQUIRE(opDivider.check() == true);
+    REQUIRE(opDivider.checkAndIncrement() == true);
+    REQUIRE(opDivider.getCounter() == 1);
+    REQUIRE(opDivider.check() == true);
+    REQUIRE(opDivider.checkAndIncrement() == true);
+    REQUIRE(opDivider.checkAndIncrement() == true);
+
+    opDivider.setDivider(0);
+    REQUIRE(opDivider.getCounter() == 1);
+    REQUIRE(opDivider.checkAndIncrement() == true);
+    REQUIRE(opDivider.getCounter() == 1);
+    REQUIRE(opDivider.checkAndIncrement() == true);
+    REQUIRE(opDivider.checkAndIncrement() == true);
+
+    opDivider.setDivider(2);
+    opDivider.resetCounter();
+    REQUIRE(opDivider.getDivider() == 2);
+    REQUIRE(opDivider.getCounter() == 1);
+    REQUIRE(opDivider.check() == false);
+    REQUIRE(opDivider.checkAndIncrement() == false);
+    REQUIRE(opDivider.getCounter() == 2);
+    REQUIRE(opDivider.check() == true);
+    REQUIRE(opDivider.checkAndIncrement() == true);
+    REQUIRE(opDivider.getCounter() == 1);
+    REQUIRE(opDivider.check() == false);
+    REQUIRE(opDivider.checkAndIncrement() == false);
+    REQUIRE(opDivider.getCounter() == 2);
+    REQUIRE(opDivider.checkAndIncrement() == true);
+    REQUIRE(opDivider.checkAndIncrement() == false);
+    REQUIRE(opDivider.checkAndIncrement() == true);
+    REQUIRE(opDivider.checkAndIncrement() == false);
+
+    opDivider.setDivider(3);
+    opDivider.resetCounter();
+    REQUIRE(opDivider.checkAndIncrement() == false);
+    REQUIRE(opDivider.checkAndIncrement() == false);
+    REQUIRE(opDivider.getCounter() == 3);
+    REQUIRE(opDivider.checkAndIncrement() == true);
+    REQUIRE(opDivider.getCounter() == 1);
+    REQUIRE(opDivider.checkAndIncrement() == false);
+
+    auto opDividerNonResetting = PeriodicOperationDivider(2, false);
+    REQUIRE(opDividerNonResetting.getCounter() == 1);
+    REQUIRE(opDividerNonResetting.check() == false);
+    REQUIRE(opDividerNonResetting.checkAndIncrement() == false);
+    REQUIRE(opDividerNonResetting.getCounter() == 2);
+    REQUIRE(opDividerNonResetting.check() == true);
+    REQUIRE(opDividerNonResetting.checkAndIncrement() == true);
+    REQUIRE(opDividerNonResetting.getCounter() == 3);
+    REQUIRE(opDividerNonResetting.checkAndIncrement() == true);
+    REQUIRE(opDividerNonResetting.getCounter() == 4);
+    opDividerNonResetting.resetCounter();
+    REQUIRE(opDividerNonResetting.getCounter() == 1);
+    REQUIRE(opDividerNonResetting.check() == false);
+    REQUIRE(opDividerNonResetting.checkAndIncrement() == false);
+    REQUIRE(opDividerNonResetting.getCounter() == 2);
+}