From 7c28ad465e18f215e44ba7d5e2fab5025a62ffad Mon Sep 17 00:00:00 2001 From: Leon Oostrum Date: Wed, 20 Nov 2024 13:13:02 +0100 Subject: [PATCH 1/8] When a marker timeout is reached, remove the marker from the queue also when no dumpfile is in use --- host/src/PowerSensor.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/host/src/PowerSensor.cc b/host/src/PowerSensor.cc index c852116..6b22e6b 100644 --- a/host/src/PowerSensor.cc +++ b/host/src/PowerSensor.cc @@ -351,8 +351,8 @@ void PowerSensor::writeMarker() { if (dumpFile != nullptr) { std::unique_lock lock(dumpFileMutex); *dumpFile << "M " << markers.front() << std::endl; - markers.pop(); } + markers.pop(); } /** From f5be3047a947057ab177ea36f1eeda94766ef509 Mon Sep 17 00:00:00 2001 From: Leon Oostrum Date: Wed, 20 Nov 2024 13:15:39 +0100 Subject: [PATCH 2/8] Fix disappearing markers Setting the marker bit is moved from the IRQ handler to the main loop. The data to send is copied to avoid the IRQ handler overwriting it before sending --- device/PowerSensor/PowerSensor.ino | 11 ++++++++++- .../device_specific/BLACKPILL_F401CC_F411CE.hpp | 12 +++--------- device/PowerSensor/device_specific/DISCO_F407VG.hpp | 11 +++-------- 3 files changed, 16 insertions(+), 18 deletions(-) diff --git a/device/PowerSensor/PowerSensor.ino b/device/PowerSensor/PowerSensor.ino index 43e34f4..1f3241c 100644 --- a/device/PowerSensor/PowerSensor.ino +++ b/device/PowerSensor/PowerSensor.ino @@ -411,7 +411,16 @@ void setup() { void loop() { if (sendData) { - Serial.write(serialData, sizeof(serialData)); + // copy serialData to avoid overwrite by next values in IRQ handler + uint8_t serialDataToSend[(SENSORS + 1) * 2]; // 16b per sensor and 16b for timestamp + memcpy(serialDataToSend, serialData, sizeof(serialData)); + if (sendMarkers > 0) { + // set marker bit in sensor data of sensor 0. First 2 bytes are timestamp + // marker bit is 6th bit from the right in second byte of sensor data + serialDataToSend[3] |= 1 << 6; + sendMarkers--; + } + Serial.write(serialDataToSend, sizeof(serialDataToSend)); sendData = false; } // check for serial events diff --git a/device/PowerSensor/device_specific/BLACKPILL_F401CC_F411CE.hpp b/device/PowerSensor/device_specific/BLACKPILL_F401CC_F411CE.hpp index 975d38c..d3fd0aa 100644 --- a/device/PowerSensor/device_specific/BLACKPILL_F401CC_F411CE.hpp +++ b/device/PowerSensor/device_specific/BLACKPILL_F401CC_F411CE.hpp @@ -106,7 +106,7 @@ extern "C" void DMA2_Stream0_IRQHandler() { * can only be set for sensor 0 * so the timestamp packets are * 1 111 TTTT, where T are the upper 4 bits of the timestamp - * 0 1 TTTTTT, where T are the lower 4 bits of the timestamp + * 0 1 TTTTTT, where T are the lower 6 bits of the timestamp */ uint16_t t = micros(); serialData[0] = (0b1111 << 4) | ((t & 0x3C0) >> 6); @@ -124,22 +124,16 @@ extern "C" void DMA2_Stream0_IRQHandler() { // store in sensorValues for display purposes sensorLevels[i] = level; #endif - // determine whether or not to send marker - // marker is always sent with sensor 0 - bool sendMarkerNext = (i == 0) && (sendMarkers > 0); - if (sendMarkerNext) { - sendMarkers--; - } // add metadata to remaining bits: 2 bytes available with 10b sensor value // First byte: 1 iii aaaa // where iii is the sensor id, a are the upper 4 bits of the level serialData[2*i + 2] = ((i & 0x7) << 4) | ((level & 0x3C0) >> 6) | (1 << 7); // Second byte: 0 m bbbbbb // where m is the marker bit, b are the lower 6 bits of the level - serialData[2*i + 3] = ((sendMarkerNext << 6) | (level & 0x3F)) & ~(1 << 7); + // the marker bit is set by the main loop, always zero here + serialData[2*i + 3] = (level & 0x3F) & ~(11 << 6); counter++; - sendMarkerNext = false; } // finally reset the sampling counter and trigger the sending of data in the main loop currentSample = 0; diff --git a/device/PowerSensor/device_specific/DISCO_F407VG.hpp b/device/PowerSensor/device_specific/DISCO_F407VG.hpp index 2f55ceb..5849c69 100644 --- a/device/PowerSensor/device_specific/DISCO_F407VG.hpp +++ b/device/PowerSensor/device_specific/DISCO_F407VG.hpp @@ -99,7 +99,7 @@ extern "C" void DMA2_Stream0_IRQHandler() { * can only be set for sensor 0 * so the timestamp packets are * 1 111 TTTT, where T are the upper 4 bits of the timestamp - * 0 1 TTTTTT, where T are the lower 4 bits of the timestamp + * 0 1 TTTTTT, where T are the lower 6 bits of the timestamp */ uint16_t t = micros(); serialData[0] = (0b1111 << 4) | ((t & 0x3C0) >> 6); @@ -118,19 +118,14 @@ extern "C" void DMA2_Stream0_IRQHandler() { // store in sensorValues for display purposes sensorLevels[i] = level; #endif - // determine whether or not to send marker - // marker is always sent with sensor 0 - bool sendMarkerNext = (i == 0) && (sendMarkers > 0); - if (sendMarkerNext) { - sendMarkers--; - } // add metadata to remaining bits: 2 bytes available with 10b sensor value // First byte: 1 iii aaaa // where iii is the sensor id, a are the upper 4 bits of the level serialData[2*i + 2] = ((i & 0x7) << 4) | ((level & 0x3C0) >> 6) | (1 << 7); // Second byte: 0 m bbbbbb // where m is the marker bit, b are the lower 6 bits of the level - serialData[2*i + 3] = ((sendMarkerNext << 6) | (level & 0x3F)) & ~(1 << 7); + // the marker bit is set by the main loop, always zero here + serialData[2*i + 3] = (level & 0x3F) & ~(1 << 7); counter++; sendMarkerNext = false; } From 6792a5b20bce36a812c2df8857f74b897494e962 Mon Sep 17 00:00:00 2001 From: Leon Oostrum Date: Wed, 20 Nov 2024 13:33:18 +0100 Subject: [PATCH 3/8] Remove leftover marker reference in F407-specific file --- device/PowerSensor/device_specific/DISCO_F407VG.hpp | 1 - 1 file changed, 1 deletion(-) diff --git a/device/PowerSensor/device_specific/DISCO_F407VG.hpp b/device/PowerSensor/device_specific/DISCO_F407VG.hpp index 5849c69..d652965 100644 --- a/device/PowerSensor/device_specific/DISCO_F407VG.hpp +++ b/device/PowerSensor/device_specific/DISCO_F407VG.hpp @@ -127,7 +127,6 @@ extern "C" void DMA2_Stream0_IRQHandler() { // the marker bit is set by the main loop, always zero here serialData[2*i + 3] = (level & 0x3F) & ~(1 << 7); counter++; - sendMarkerNext = false; } // trigger sending data to host if enabled From 7f16b9145e68bc6a4e260d7daf85648d61e64098 Mon Sep 17 00:00:00 2001 From: Leon Oostrum Date: Wed, 20 Nov 2024 13:39:41 +0100 Subject: [PATCH 4/8] simplify declaration of serialDataToSend --- device/PowerSensor/PowerSensor.ino | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/device/PowerSensor/PowerSensor.ino b/device/PowerSensor/PowerSensor.ino index 1f3241c..b673d75 100644 --- a/device/PowerSensor/PowerSensor.ino +++ b/device/PowerSensor/PowerSensor.ino @@ -412,7 +412,7 @@ void setup() { void loop() { if (sendData) { // copy serialData to avoid overwrite by next values in IRQ handler - uint8_t serialDataToSend[(SENSORS + 1) * 2]; // 16b per sensor and 16b for timestamp + uint8_t serialDataToSend[sizeof(SerialData)]; memcpy(serialDataToSend, serialData, sizeof(serialData)); if (sendMarkers > 0) { // set marker bit in sensor data of sensor 0. First 2 bytes are timestamp From d85956f193e73d813037f4bdf0a394f0bb9542a5 Mon Sep 17 00:00:00 2001 From: Leon Oostrum Date: Wed, 20 Nov 2024 14:04:31 +0100 Subject: [PATCH 5/8] Fix typo --- device/PowerSensor/PowerSensor.ino | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/device/PowerSensor/PowerSensor.ino b/device/PowerSensor/PowerSensor.ino index b673d75..5bf8e11 100644 --- a/device/PowerSensor/PowerSensor.ino +++ b/device/PowerSensor/PowerSensor.ino @@ -412,7 +412,7 @@ void setup() { void loop() { if (sendData) { // copy serialData to avoid overwrite by next values in IRQ handler - uint8_t serialDataToSend[sizeof(SerialData)]; + uint8_t serialDataToSend[sizeof(serialData)]; memcpy(serialDataToSend, serialData, sizeof(serialData)); if (sendMarkers > 0) { // set marker bit in sensor data of sensor 0. First 2 bytes are timestamp From e95c174e0b9763e21744ac8e504249d3dbe4df50 Mon Sep 17 00:00:00 2001 From: Leon Oostrum Date: Wed, 20 Nov 2024 14:07:22 +0100 Subject: [PATCH 6/8] lock dumpfile earlier to avoid segfault --- host/src/PowerSensor.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/host/src/PowerSensor.cc b/host/src/PowerSensor.cc index 6b22e6b..c995ec8 100644 --- a/host/src/PowerSensor.cc +++ b/host/src/PowerSensor.cc @@ -348,8 +348,8 @@ void PowerSensor::mark(char name) { * */ void PowerSensor::writeMarker() { + std::unique_lock lock(dumpFileMutex); if (dumpFile != nullptr) { - std::unique_lock lock(dumpFileMutex); *dumpFile << "M " << markers.front() << std::endl; } markers.pop(); @@ -474,10 +474,10 @@ void PowerSensor::dump(std::string dumpFileName) { * */ void PowerSensor::dumpCurrentWattToFile() { + std::unique_lock lock(dumpFileMutex); if (dumpFile == nullptr) { return; } - std::unique_lock lock(dumpFileMutex); double totalWatt = 0; auto time = std::chrono::high_resolution_clock::now(); static auto previousTime = startTime; From 1d2c7f2db47f4bcfc3efd27257657b30fc09dc52 Mon Sep 17 00:00:00 2001 From: Leon Oostrum Date: Wed, 20 Nov 2024 15:22:42 +0100 Subject: [PATCH 7/8] Make marker char inline with sensor measurements, it replaces the default S --- host/include/PowerSensor.hpp | 3 +-- host/src/PowerSensor.cc | 28 +++++++++------------------- 2 files changed, 10 insertions(+), 21 deletions(-) diff --git a/host/include/PowerSensor.hpp b/host/include/PowerSensor.hpp index 7e432c2..a2bbabd 100644 --- a/host/include/PowerSensor.hpp +++ b/host/include/PowerSensor.hpp @@ -81,7 +81,6 @@ class PowerSensor { int pipe_fd; int openDevice(std::string device); std::queue markers; - void writeMarker(); void waitForMarkers(int timeout=500); void initializeSensorPairs(); @@ -93,7 +92,7 @@ class PowerSensor { bool readLevelFromDevice(unsigned int* sensorNumber, uint16_t* level, unsigned int* marker); std::unique_ptr dumpFile; - void dumpCurrentWattToFile(); + void dumpCurrentWattToFile(const char markerChar); Semaphore threadStarted; std::thread* thread; diff --git a/host/src/PowerSensor.cc b/host/src/PowerSensor.cc index c995ec8..2183799 100644 --- a/host/src/PowerSensor.cc +++ b/host/src/PowerSensor.cc @@ -343,18 +343,6 @@ void PowerSensor::mark(char name) { writeCharToDevice('M'); } -/** - * @brief Write marker character to output file - * - */ -void PowerSensor::writeMarker() { - std::unique_lock lock(dumpFileMutex); - if (dumpFile != nullptr) { - *dumpFile << "M " << markers.front() << std::endl; - } - markers.pop(); -} - /** * @brief Wait for all markers to be written to the dump file * @@ -405,12 +393,14 @@ void PowerSensor::IOThread() { sensorsRead = 0; updateSensorPairs(); + char markerChar = 'S'; + if (marker != 0) { + markerChar = markers.front(); + markers.pop(); + marker = 0; + } if (dumpFile != nullptr) { - if (marker != 0) { - writeMarker(); - marker = 0; - } - dumpCurrentWattToFile(); + dumpCurrentWattToFile(markerChar); } } } @@ -473,7 +463,7 @@ void PowerSensor::dump(std::string dumpFileName) { * @brief Write sensor values to the output file * */ -void PowerSensor::dumpCurrentWattToFile() { +void PowerSensor::dumpCurrentWattToFile(const char markerChar) { std::unique_lock lock(dumpFileMutex); if (dumpFile == nullptr) { return; @@ -482,7 +472,7 @@ void PowerSensor::dumpCurrentWattToFile() { auto time = std::chrono::high_resolution_clock::now(); static auto previousTime = startTime; - *dumpFile << "S " << elapsedSeconds(startTime, time); + *dumpFile << markerChar << " " << elapsedSeconds(startTime, time); *dumpFile << ' ' << static_cast(1e6 * elapsedSeconds(previousTime, time)); *dumpFile << ' ' << timestamp; previousTime = time; From f440a9c3c5cae27a43950533b121ec3f2d029fa5 Mon Sep 17 00:00:00 2001 From: Leon Oostrum Date: Wed, 20 Nov 2024 16:24:10 +0100 Subject: [PATCH 8/8] single-quoted space for consistency --- host/src/PowerSensor.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/host/src/PowerSensor.cc b/host/src/PowerSensor.cc index 2183799..2926729 100644 --- a/host/src/PowerSensor.cc +++ b/host/src/PowerSensor.cc @@ -472,7 +472,7 @@ void PowerSensor::dumpCurrentWattToFile(const char markerChar) { auto time = std::chrono::high_resolution_clock::now(); static auto previousTime = startTime; - *dumpFile << markerChar << " " << elapsedSeconds(startTime, time); + *dumpFile << markerChar << ' ' << elapsedSeconds(startTime, time); *dumpFile << ' ' << static_cast(1e6 * elapsedSeconds(previousTime, time)); *dumpFile << ' ' << timestamp; previousTime = time;