Merge pull request #189 from nlesc-recruit/update-markers

Make marker use more robust

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[sizeof(serialData)];
+    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

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;

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,21 +118,15 @@ 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;
   }
 
   // trigger sending data to host if enabled

host/include/PowerSensor.hpp

@@ -81,7 +81,6 @@ class PowerSensor {
     int pipe_fd;
     int openDevice(std::string device);
     std::queue<char> 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<std::ofstream> dumpFile;
-    void dumpCurrentWattToFile();
+    void dumpCurrentWattToFile(const char markerChar);
 
     Semaphore threadStarted;
     std::thread* thread;

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() {
-  if (dumpFile != nullptr) {
-    std::unique_lock<std::mutex> lock(dumpFileMutex);
-    *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,16 +463,16 @@ 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<std::mutex> lock(dumpFileMutex);
   if (dumpFile == nullptr) {
     return;
   }
-  std::unique_lock<std::mutex> lock(dumpFileMutex);
   double totalWatt = 0;
   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<int>(1e6 * elapsedSeconds(previousTime, time));
   *dumpFile << ' ' << timestamp;
   previousTime = time;