Skip to content

Commit

Permalink
publishers/EnviroDIYPublisher: move log buffer logic to its own class
Browse files Browse the repository at this point in the history
Clean up and make maintenance easier
  • Loading branch information
tpwrules committed Sep 13, 2023
1 parent 793bbb9 commit 74cac0d
Show file tree
Hide file tree
Showing 4 changed files with 272 additions and 74 deletions.
90 changes: 90 additions & 0 deletions src/LogBuffer.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
/**
* @file LogBuffer.cpp
* @copyright 2023 Thomas Watson
* Part of the EnviroDIY ModularSensors library for Arduino
* @author Thomas Watson <[email protected]>
*
* @brief Implements the LogBuffer class.
*
* This class buffers logged timestamps and variable values for transmission.
*/
#include "LogBuffer.h"

#include <string.h>

// Constructor
LogBuffer::LogBuffer() {}
// Destructor
LogBuffer::~LogBuffer() {}

void LogBuffer::setNumVariables(uint8_t numVariables_) {
// each record is one uint32_t to hold the timestamp, plus N floats to hold
// each variable's value
recordSize = sizeof(uint32_t) + sizeof(float) * numVariables_;
numVariables = numVariables_;

// this scrambles all the data in the buffer so clear it out
numRecords = 0;
}

void LogBuffer::clear(void) {
// clear out the buffer
numRecords = 0;
}

uint8_t LogBuffer::getNumVariables(void) {
return numVariables;
}

int LogBuffer::getNumRecords(void) {
return numRecords;
}

int LogBuffer::addRecord(uint32_t timestamp) {
int record = numRecords;
// compute position of the new record's timestamp in the buffer
// (the timestamp is the first data in the record)
size_t pos = record * recordSize;
// verify we have sufficient space for the record and bail if not
if (MS_LOG_DATA_BUFFER_SIZE - pos < recordSize) { return -1; }

// write the timestamp to the record
memcpy(static_cast<void*>(&dataBuffer[pos]), static_cast<void*>(&timestamp),
sizeof(uint32_t));
numRecords += 1; // just added another record

return record;
}

void LogBuffer::setRecordValue(int record, uint8_t variable, float value) {
// compute position of this value in the buffer
size_t pos = record * recordSize + sizeof(uint32_t) +
variable * sizeof(float);

// write the value to the record
memcpy(static_cast<void*>(&dataBuffer[pos]), static_cast<void*>(&value),
sizeof(float));
}

uint32_t LogBuffer::getRecordTimestamp(int record) {
// read the timestamp from the record (which is the first data in it)
uint32_t timestamp;
memcpy(static_cast<void*>(&timestamp),
static_cast<void*>(&dataBuffer[record * recordSize]),
sizeof(uint32_t));

return timestamp;
}

float LogBuffer::getRecordValue(int record, uint8_t variable) {
// compute position of this value in the buffer
size_t pos = record * recordSize + sizeof(uint32_t) +
variable * sizeof(float);

// read the value from the record
float value;
memcpy(static_cast<void*>(&value), static_cast<void*>(&dataBuffer[pos]),
sizeof(float));

return value;
}
138 changes: 138 additions & 0 deletions src/LogBuffer.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
/**
* @file LogBuffer.cpp
* @copyright 2023 Thomas Watson
* Part of the EnviroDIY ModularSensors library for Arduino
* @author Thomas Watson <[email protected]>
*
* @brief Implements the LogBuffer class.
*
* This class buffers logged timestamps and variable values for transmission.
*/

// Header Guards
#ifndef SRC_LOGBUFFER_H_
#define SRC_LOGBUFFER_H_

/**
* @def MS_LOG_DATA_BUFFER_SIZE
* @brief Log Data Buffer
*
* This determines how much RAM is reserved to buffer log records before
* transmission. Each record consumes 4 bytes for the timestamp plus 4 bytes
* for each logged variable. Increasing this value too far can crash the
* device! The number of log records buffered is controlled by sendEveryX.
*
* This can be changed by setting the build flag MS_LOG_DATA_BUFFER_SIZE when
* compiling. 8192 bytes is a safe value for the Mayfly 1.1 with six variables.
*/
#ifndef MS_LOG_DATA_BUFFER_SIZE
#define MS_LOG_DATA_BUFFER_SIZE 8192
#endif

#include <stddef.h>
#include <inttypes.h>

/**
* @brief This class buffers logged timestamps and variable values for
* transmission. The log is divided into a number of records. Each record
* stores the timestamp of the record as a uint32_t, then the value of each
* variable as a float at that time.
*/
class LogBuffer {
public:
/**
* @brief Constructs a new empty buffer which stores no variables or values.
*/
LogBuffer();
/**
* @brief Destroys the buffer.
*/
virtual ~LogBuffer();

/**
* @brief Sets the number of variables the buffer will store in each record.
* Clears the buffer as a side effect.
*
* @param[in] numVariables_ The number of variables to store.
*/
void setNumVariables(uint8_t numVariables_);

/**
* @brief Gets the number of variables that will be stored in each record.
*
* @return The variable count.
*/
uint8_t getNumVariables(void);

/**
* @brief Clears all records from the log.
*/
void clear(void);

/**
* @brief Gets the number of records currently in the log.
*
* @return The number of records.
*/
int getNumRecords(void);

/**
* @brief Adds a new record with the given timestamp.
*
* @param[in] timestamp The timestamp
*
* @return Index of the new record, or -1 if there was no space.
*/
int addRecord(uint32_t timestamp);

/**
* @brief Sets the value of a particular variable in a particular record.
*
* @param[in] record The record
* @param[in] variable The variable
* @param[in] value The value
*/
void setRecordValue(int record, uint8_t variable, float value);

/**
* @brief Gets the timestamp of a particular record.
*
* @param[in] record The record
*
* @return The record's timestamp.
*/
uint32_t getRecordTimestamp(int record);

/**
* @brief Gets the value of a particular vaiable in a particular record.
*
* @param[in] record The record
* @param[in] variable The variable
*
* @return The variable's value.
*/
float getRecordValue(int record, uint8_t variable);

protected:
/**
* @brief Buffer which stores the log data.
*/
uint8_t dataBuffer[MS_LOG_DATA_BUFFER_SIZE];

/**
* @brief Number of records currently in the buffer.
*/
int numRecords;

/**
* @brief Size in bytes of each record in the buffer.
*/
size_t recordSize;

/**
* @brief Number of variables stored in each record in the buffer.
*/
uint8_t numVariables;
};

#endif // SRC_LOGBUFFER_H_
Loading

4 comments on commit 74cac0d

@SRGDamia1
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tpwrules Shouldn't the log buffer be a property of the logger itself so it can be used by all of the publishers instead of assigning one large buffer for each publisher?

@SRGDamia1
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tpwrules I'm having issues with the set of the number of variables from the constructor of the EnviroDIY publisher. It's being constructed with 0 variables, despite being declared after the variable array and the logger object.

@SRGDamia1
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tpwrules You're right; moving the buffer to the logger would be much more complex than I first imagined.

Problems with cross-publisher shared buffers:

  • A publish to one endpoint might succeed and to another fails
  • Endpoints have different ways of batching data

Problems with static buffers per publisher type:

  • my initialization issue
  • what if someone wanted to have two of the same publisher - ie, to two thingspeak channels

In @neilh10's work, with a file-based queue, he solves this by having a different queue file for each publisher.

@neilh10
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to me that the file based method would be more extensible than storing in ram.
I've orginated an issue to have a higher level view - seems to me thats an easier way of discussing it EnviroDIY#454

My specific comments, repeated in above issue are

For proposed implementation of ::xxPublisher with a logbuffer based in ram memory, filled every period by a reading action and spoofing a 201 response, it’s breaking the layered model and the meaning of 201. By spoofing a 201, I would expect the implementation to be guarenting the meaning of the 201 - that it will be delivered to the server.
For the period that the data is stored in the logbuffer, which for 15minutes sampling if it was 4records is one hour, and 8 records is two hours, If someone walks up to the system and plugs in a USB monitor, if there is a reset watchdog or maintenance action (reset) – the last set of readings stored in the ram buffer are lost.

Please sign in to comment.