From 81271df4fe482105baf03d9a73af789f29aab567 Mon Sep 17 00:00:00 2001
From: Dimitrios Stoupis <dimitris.apple@gmail.com>
Date: Wed, 21 Nov 2018 21:15:58 +0000
Subject: [PATCH] Implement almost all code review suggestions
---
CMakeLists.txt | 4 ++--
inc/Message.hpp | 7 +++----
...emMangService.hpp => MemoryManagementService.hpp} | 5 +++--
...emMangService.cpp => MemoryManagementService.cpp} | 12 +++++++-----
src/main.cpp | 2 +-
...emMangService.cpp => MemoryManagementService.cpp} | 2 +-
6 files changed, 17 insertions(+), 15 deletions(-)
rename inc/Services/{MemMangService.hpp => MemoryManagementService.hpp} (91%)
rename src/Services/{MemMangService.cpp => MemoryManagementService.cpp} (91%)
rename test/Services/{MemMangService.cpp => MemoryManagementService.cpp} (98%)
diff --git a/CMakeLists.txt b/CMakeLists.txt
index f667453e..810fb26f 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -14,10 +14,10 @@ add_custom_target(check
WORKING_DIRECTORY "${PROJECT_SOURCE_DIR}/ci")
# Specify the .cpp files for the executables
-add_executable(ecss_services src/main.cpp src/Message.cpp src/Service.cpp src/Services/TestService.cpp src/Services/MemMangService.cpp)
+add_executable(ecss_services src/main.cpp src/Message.cpp src/Service.cpp src/Services/TestService.cpp src/Services/MemoryManagementService.cpp)
IF(EXISTS "${PROJECT_SOURCE_DIR}/lib/Catch2/CMakeLists.txt")
add_subdirectory(lib/Catch2)
-add_executable(tests src/Message.cpp src/Services/TestService.cpp src/Services/MemMangService.cpp test/tests.cpp test/Message.cpp test/TestPlatform.cpp test/Services/TestService.cpp test/Services/MemMangService.cpp)
+add_executable(tests src/Message.cpp src/Services/TestService.cpp src/Services/MemoryManagementService.cpp test/tests.cpp test/Message.cpp test/TestPlatform.cpp test/Services/TestService.cpp test/Services/MemoryManagementService.cpp)
target_link_libraries(tests Catch2::Catch2)
ENDIF()
diff --git a/inc/Message.hpp b/inc/Message.hpp
index 53bfb031..ce2c5944 100644
--- a/inc/Message.hpp
+++ b/inc/Message.hpp
@@ -194,7 +194,7 @@ public:
*/
void appendUint64(uint64_t value) {
appendWord(static_cast<uint32_t >(value >> 32));
- return appendWord(static_cast<uint32_t >(value));
+ appendWord(static_cast<uint32_t >(value));
}
/**
@@ -383,16 +383,15 @@ public:
/**
* Fetches a N-byte string from the current position in the message
*
- *
* PTC = 7, PFC = 0
*/
- void readOctetString(uint8_t *byteString, uint16_t size) {
+ void readOctetString(uint8_t *byteString, uint16_t size) {
assert(size < ECSS_MAX_STRING_SIZE);
for (std::size_t i = 0; i < size; i++) {
byteString[i] = readByte();
}
- }
+ }
/**
* Reset the message reading status, and start reading data from it again
diff --git a/inc/Services/MemMangService.hpp b/inc/Services/MemoryManagementService.hpp
similarity index 91%
rename from inc/Services/MemMangService.hpp
rename to inc/Services/MemoryManagementService.hpp
index 3f4030b6..1d2de9be 100644
--- a/inc/Services/MemMangService.hpp
+++ b/inc/Services/MemoryManagementService.hpp
@@ -25,10 +25,11 @@ public:
*/
class RawDataMemoryManagement {
private:
- MemoryManagementService *mainService; // Used to access main class's members
+ MemoryManagementService &mainService; // Used to access main class's members
public:
- explicit RawDataMemoryManagement(MemoryManagementService *parent);
+ explicit RawDataMemoryManagement(MemoryManagementService &parent);
+
/**
* TC[6,2] load raw values to memory
*
diff --git a/src/Services/MemMangService.cpp b/src/Services/MemoryManagementService.cpp
similarity index 91%
rename from src/Services/MemMangService.cpp
rename to src/Services/MemoryManagementService.cpp
index d5d0bb7e..72c086a5 100644
--- a/src/Services/MemMangService.cpp
+++ b/src/Services/MemoryManagementService.cpp
@@ -1,14 +1,14 @@
-#include "Services/MemMangService.hpp"
+#include "Services/MemoryManagementService.hpp"
#include <iostream>
#include <cerrno>
// Define the constructors for the classes
-MemoryManagementService::MemoryManagementService() : rawDataMemorySubservice(this) {
+MemoryManagementService::MemoryManagementService() : rawDataMemorySubservice(*this) {
serviceType = 6;
}
MemoryManagementService::RawDataMemoryManagement::RawDataMemoryManagement(
- MemoryManagementService *parent) : mainService(parent) {}
+ MemoryManagementService &parent) : mainService(parent) {}
// Function declarations for the raw data memory management subservice
@@ -38,6 +38,7 @@ void MemoryManagementService::RawDataMemoryManagement::loadRawData(Message &requ
// Allocate more array space if needed
if (allocatedLength < dataLength) {
+ // todo: In embedded implementation use the malloc, due to FreeRTOS constraints
tempMemory = static_cast<uint8_t *>(realloc(readData, dataLength));
if (tempMemory == nullptr) {
// todo: Add error logging and reporting
@@ -68,7 +69,7 @@ void MemoryManagementService::RawDataMemoryManagement::dumpRawData(Message &requ
assert(request.messageType == 5);
// Create the report message object of telemetry message subtype 6
- Message report = mainService->createTM(6);
+ Message report = mainService.createTM(6);
// Variable declaration
uint8_t *readData = nullptr, *tempMemory = nullptr;; // Pointer to store the read data
@@ -89,6 +90,7 @@ void MemoryManagementService::RawDataMemoryManagement::dumpRawData(Message &requ
// Allocate more array space if needed
if (allocatedLength < readLength) {
+ // todo: In embedded implementation use the malloc, due to FreeRTOS constraints
tempMemory = static_cast<uint8_t *>(realloc(readData, readLength));
if (tempMemory == nullptr) {
// todo: Add error logging and reporting
@@ -111,7 +113,7 @@ void MemoryManagementService::RawDataMemoryManagement::dumpRawData(Message &requ
}
// todo: implement and append the checksum part of the reporting packet
- mainService->storeMessage(report); // Save the report message
+ mainService.storeMessage(report); // Save the report message
request.resetRead(); // Reset the reading count
free(readData); // Free the allocated memory
}
diff --git a/src/main.cpp b/src/main.cpp
index 108bd545..d1a628e2 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -1,7 +1,7 @@
#include <iostream>
#include <Services/TestService.hpp>
#include "Message.hpp"
-#include "Services/MemMangService.hpp"
+#include "Services/MemoryManagementService.hpp"
int main() {
Message packet = Message(0, 0, Message::TC, 1);
diff --git a/test/Services/MemMangService.cpp b/test/Services/MemoryManagementService.cpp
similarity index 98%
rename from test/Services/MemMangService.cpp
rename to test/Services/MemoryManagementService.cpp
index 8d7f63f4..824866cb 100644
--- a/test/Services/MemMangService.cpp
+++ b/test/Services/MemoryManagementService.cpp
@@ -1,5 +1,5 @@
#include <catch2/catch.hpp>
-#include <Services/MemMangService.hpp>
+#include <Services/MemoryManagementService.hpp>
#include <Message.hpp>
#include "ServiceTests.hpp"
--
GitLab