mirror of https://github.com/EspoTek/Labrador.git
Isobuffer refactor - part 2 (#65)
* Split isoBuffer::maybeOutputSampleToFile into two functions * reorder isoBuffer data members. clean up comments and formatting. clean isoBuffer::writeBuffer * Remove my name from a comment * Use type-safe Qt connect. Move comments. change IO column comparison. * Give isoBuffer the proper ringBuffer behaviour * Change the behaviour of isoBuffer::readBuffer; Change comments to reflect this. * Fix a silly mistake in isobuffer.cpp (compiler warnings are awesome) * Modify isoBuffer::readBuffer * add destructor to isoBuffer, reorder initializer list. isobuffer.cpp now compiles with no warnings. * Change isoBuffer::m_buffer to be a unique_ptr * Change isoBuffer::m_stopDecoding to isoBuffer::m_isDecoding * Refactor isoBuffer::readBuffer to use unique_ptr. Currently a memory leak * Change signature of isoBuffer::readBuffer to return unique_ptr. adapt isoDriver acordingly. * Clean up comments on isobuffer .h and .cpp * Incorporate review comments: formatting, explicit unique_ptr, spelling
This commit is contained in:
Sebastián Mestre
Chris Esposito
parent
31f68faf4e
commit
57c7f5f61f
|
|
@ -1,9 +1,13 @@
|
|||
#include "isobuffer.h"
|
||||
|
||||
#include <algorithm>
|
||||
|
||||
#include "isodriver.h"
|
||||
#include "uartstyledecoder.h"
|
||||
|
||||
namespace {
|
||||
static char const * fileHeaderFormat =
|
||||
namespace
|
||||
{
|
||||
constexpr char const* fileHeaderFormat =
|
||||
"EspoTek Labrador DAQ V1.0 Output File\n"
|
||||
"Averaging = %d\n"
|
||||
"Mode = %d\n";
|
||||
|
|
@ -23,18 +27,20 @@ namespace {
|
|||
|
||||
isoBuffer::isoBuffer(QWidget* parent, int bufferLen, isoDriver* caller, unsigned char channel_value)
|
||||
: QWidget(parent)
|
||||
, m_buffer((short*)calloc(bufferLen*2, sizeof(short)))
|
||||
, m_channel(channel_value)
|
||||
, m_buffer(std::make_unique<short[]>(bufferLen*2))
|
||||
, m_bufferEnd(bufferLen-1)
|
||||
, m_samplesPerSecond(bufferLen/21.0/375*VALID_DATA_PER_375)
|
||||
, m_sampleRate_bit(bufferLen/21.0/375*VALID_DATA_PER_375*8)
|
||||
, m_virtualParent(caller)
|
||||
, m_channel(channel_value)
|
||||
{
|
||||
}
|
||||
|
||||
// NOTE: the length of half of the allocated buffer is m_bufferEnd+1
|
||||
void isoBuffer::insertIntoBuffer(short item)
|
||||
{
|
||||
m_buffer[m_back] = item;
|
||||
m_buffer[m_back+m_bufferEnd+1] = item;
|
||||
m_back++;
|
||||
m_insertedCount++;
|
||||
|
||||
|
|
@ -51,49 +57,58 @@ void isoBuffer::insertIntoBuffer(short item)
|
|||
|
||||
short isoBuffer::bufferAt(int idx) const
|
||||
{
|
||||
return m_buffer[m_back - idx];
|
||||
// NOTE: this is only correct if idx < m_insertedCount
|
||||
return m_buffer[m_back + (m_bufferEnd+1) - idx];
|
||||
}
|
||||
|
||||
bool isoBuffer::maybeOutputSampleToFile(double convertedSample)
|
||||
void isoBuffer::outputSampleToFile(double averageSample)
|
||||
{
|
||||
/*
|
||||
* This function adds a sample to an accumulator and bumps the sample count.
|
||||
* After the sample count hits some threshold, the accumulated sample is
|
||||
* outputted to a file. If this 'saturates' the file, then fileIO is disabled.
|
||||
*/
|
||||
m_average_sample_temp += convertedSample;
|
||||
m_fileIO_sampleCount++;
|
||||
|
||||
// Check to see if we can write a new sample to file
|
||||
if (m_fileIO_sampleCount == m_fileIO_maxIncrementedSampleValue)
|
||||
{
|
||||
char numStr[32];
|
||||
sprintf(numStr,"%7.5f, ", m_average_sample_temp/((double)m_fileIO_maxIncrementedSampleValue));
|
||||
sprintf(numStr,"%7.5f, ", averageSample);
|
||||
|
||||
m_currentFile->write(numStr);
|
||||
m_currentColumn++;
|
||||
if (m_currentColumn >= COLUMN_BREAK)
|
||||
|
||||
if (m_currentColumn == COLUMN_BREAK)
|
||||
{
|
||||
m_currentFile->write("\n");
|
||||
m_currentColumn = 0;
|
||||
}
|
||||
}
|
||||
|
||||
// Reset the average and sample count for next data point
|
||||
void isoBuffer::maybeOutputSampleToFile(double convertedSample)
|
||||
{
|
||||
/*
|
||||
* This function adds a sample to an accumulator and bumps a sample count.
|
||||
* After the sample count hits some threshold the samples are averaged
|
||||
* and the average is written to a file.
|
||||
* If this makes us hit the max. file size, then fileIO is disabled.
|
||||
*/
|
||||
|
||||
m_fileIO_sampleAccumulator += convertedSample;
|
||||
m_fileIO_sampleCount++;
|
||||
|
||||
if (m_fileIO_sampleCount == m_fileIO_sampleCountPerWrite)
|
||||
{
|
||||
double averageSample = m_fileIO_sampleAccumulator / m_fileIO_sampleCount;
|
||||
outputSampleToFile(averageSample);
|
||||
|
||||
// Reset the accumulator and sample count for next data point.
|
||||
m_fileIO_sampleAccumulator = 0;
|
||||
m_fileIO_sampleCount = 0;
|
||||
m_average_sample_temp = 0;
|
||||
|
||||
// Check to see if we've reached the max file size.
|
||||
if (m_fileIO_max_file_size != 0) // value of 0 means "no limit"
|
||||
// value of 0 means "no limit", meaning we must skip the check by returning.
|
||||
if (m_fileIO_maxFileSize == 0)
|
||||
return;
|
||||
|
||||
// 7 chars(number) + 1 char(comma) + 1 char(space) = 9 bytes/sample.
|
||||
m_fileIO_numBytesWritten += 9;
|
||||
if (m_fileIO_numBytesWritten >= m_fileIO_maxFileSize)
|
||||
{
|
||||
m_fileIO_numBytesWritten += 9; // 7 chars for the number, 1 for the comma and 1 for the space = 9 bytes per sample.
|
||||
if (m_fileIO_numBytesWritten >= m_fileIO_max_file_size)
|
||||
{
|
||||
m_fileIOEnabled = false; // Just in case signalling fails.
|
||||
fileIOinternalDisable();
|
||||
return false;
|
||||
}
|
||||
m_fileIOEnabled = false; // Just in case signalling fails.
|
||||
fileIOinternalDisable();
|
||||
}
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
template<typename T, typename Function>
|
||||
|
|
@ -109,15 +124,13 @@ void isoBuffer::writeBuffer(T* data, int len, int TOP, Function transform)
|
|||
{
|
||||
bool isUsingAC = m_channel == 1
|
||||
? m_virtualParent->AC_CH1
|
||||
: m_virtualParent->AC_CH2;
|
||||
: m_virtualParent->AC_CH2;
|
||||
|
||||
for (int i = 0; i < len; i++)
|
||||
for (int i = 0; i < len && m_fileIOEnabled; i++)
|
||||
{
|
||||
double convertedSample = sampleConvert(data[i], TOP, isUsingAC);
|
||||
|
||||
bool keepOutputting = maybeOutputSampleToFile(convertedSample);
|
||||
|
||||
if (!keepOutputting) break;
|
||||
maybeOutputSampleToFile(convertedSample);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -132,57 +145,39 @@ void isoBuffer::writeBuffer_short(short* data, int len)
|
|||
writeBuffer(data, len, 2048, [](short item) -> short {return item >> 4;});
|
||||
}
|
||||
|
||||
short* isoBuffer::readBuffer(double sampleWindow, int numSamples, bool singleBit, double delayOffset)
|
||||
std::unique_ptr<short[]> isoBuffer::readBuffer(double sampleWindow, int numSamples, bool singleBit, double delayOffset)
|
||||
{
|
||||
/* Refactor Note:
|
||||
/*
|
||||
* The expected behavior is to run backwards over the buffer with a stride
|
||||
* of timeBetweenSamples steps, and push the touched elements into readData.
|
||||
* If more elements are requested than how many are stored (1), the buffer
|
||||
* will be populated only partially. Modifying this function to return null
|
||||
* or a zero-filled buffer instead should be simple enough.
|
||||
*
|
||||
* Refactoring this function took a few passes were i made some assumptions:
|
||||
* - round() should be replaced by floor() where it was used
|
||||
* - int(floor(x)) and int(x) are equivalent (since we are always positive)
|
||||
* - free(NULL) is a no-op. This is mandated by the C standard, and virtually all
|
||||
* implementations comply. A few known exceptions are:
|
||||
* - PalmOS
|
||||
* - 3BSD
|
||||
* - UNIX 7
|
||||
* I do not know of any non-compliant somewhat modern implementations.
|
||||
*
|
||||
* The expected behavior is to cycle backwards over the buffer, taking into
|
||||
* acount only the part of the buffer that has things stored, with a stride
|
||||
* of timeBetweenSamples steps, and insert the touched elements into readData.
|
||||
*
|
||||
* ~Sebastian Mestre
|
||||
* (1) m_insertedCount < (delayOffset + sampleWindow) * m_samplesPerSecond
|
||||
*/
|
||||
const double timeBetweenSamples = sampleWindow * m_samplesPerSecond / numSamples;
|
||||
const int delaySamples = delayOffset * m_samplesPerSecond;
|
||||
|
||||
free(m_readData);
|
||||
std::unique_ptr<short[]> readData = std::make_unique<short[]>(numSamples);
|
||||
|
||||
m_readData = (short*) calloc(numSamples, sizeof(short));
|
||||
std::fill (readData.get(), readData.get() + numSamples, short(0));
|
||||
|
||||
// TODO: replace by return nullptr and add error handling upstream
|
||||
if(delaySamples+1 > m_insertedCount)
|
||||
double itr = delaySamples;
|
||||
for (int i = 0; i < numSamples && itr < m_insertedCount; i++)
|
||||
{
|
||||
return m_readData;
|
||||
}
|
||||
|
||||
double itr = delaySamples + 1;
|
||||
for (int i = 0; i < numSamples; i++)
|
||||
{
|
||||
while (itr > m_insertedCount)
|
||||
itr -= m_insertedCount;
|
||||
|
||||
m_readData[i] = bufferAt(int(itr));
|
||||
readData[i] = bufferAt(int(itr));
|
||||
|
||||
if (singleBit)
|
||||
{
|
||||
int subIdx = 8*(-itr-floor(-itr));
|
||||
m_readData[i] &= (1 << subIdx);
|
||||
readData[i] &= (1 << subIdx);
|
||||
}
|
||||
|
||||
itr += timeBetweenSamples;
|
||||
}
|
||||
|
||||
return m_readData;
|
||||
return readData;
|
||||
}
|
||||
|
||||
void isoBuffer::clearBuffer()
|
||||
|
|
@ -220,18 +215,17 @@ void isoBuffer::enableFileIO(QFile* file, int samplesToAverage, qulonglong max_f
|
|||
m_currentFile->write(headerLine);
|
||||
|
||||
// Set up the isoBuffer for DAQ
|
||||
m_fileIO_maxIncrementedSampleValue = samplesToAverage;
|
||||
m_fileIO_max_file_size = max_file_size;
|
||||
m_fileIO_maxFileSize = max_file_size;
|
||||
m_fileIO_sampleCountPerWrite = samplesToAverage;
|
||||
m_fileIO_sampleCount = 0;
|
||||
m_fileIO_sampleAccumulator = 0;
|
||||
m_fileIO_numBytesWritten = 0;
|
||||
m_average_sample_temp = 0;
|
||||
|
||||
// Enable DAQ
|
||||
m_fileIOEnabled = true;
|
||||
|
||||
qDebug("File IO enabled, averaging %d samples, max file size %lluMB", samplesToAverage, max_file_size/1000000);
|
||||
qDebug() << max_file_size;
|
||||
return;
|
||||
}
|
||||
|
||||
void isoBuffer::disableFileIO()
|
||||
|
|
@ -282,6 +276,8 @@ short isoBuffer::inverseSampleConvert(double voltageLevel, int TOP, bool AC) con
|
|||
return sample;
|
||||
}
|
||||
|
||||
// For capacitance measurement. x0, x1 and x2 are all various time points
|
||||
// used to find the RC coefficient.
|
||||
template<typename Function>
|
||||
int isoBuffer::capSample(int offset, int target, double seconds, double value, Function comp)
|
||||
{
|
||||
|
|
@ -307,7 +303,6 @@ int isoBuffer::capSample(int offset, int target, double seconds, double value, F
|
|||
return -1;
|
||||
}
|
||||
|
||||
// For capacitance measurement. x0, x1 and x2 are all various time points used to find the RC coefficient.
|
||||
int isoBuffer::cap_x0fromLast(double seconds, double vbot)
|
||||
{
|
||||
return capSample(0, kSamplesSeekingCap, seconds, vbot, fX0Comp);
|
||||
|
|
@ -328,15 +323,14 @@ void isoBuffer::serialManage(double baudRate, UartParity parity)
|
|||
if (m_decoder == NULL)
|
||||
{
|
||||
m_decoder = new uartStyleDecoder(this);
|
||||
// TODO: Look into using the type safe version of connect.
|
||||
// NOTE: I believe Qt has a type-safe version of this, without the macros and the
|
||||
// explicit signature and stuff, i think it uses member-function pointers instead.
|
||||
connect(m_decoder, SIGNAL(wireDisconnected(int)), m_virtualParent, SLOT(serialNeedsDisabling(int)));
|
||||
|
||||
connect(m_decoder, &uartStyleDecoder::wireDisconnected,
|
||||
m_virtualParent, &isoDriver::serialNeedsDisabling);
|
||||
}
|
||||
if (m_stopDecoding)
|
||||
if (!m_isDecoding)
|
||||
{
|
||||
m_decoder->updateTimer->start(CONSOLE_UPDATE_TIMER_PERIOD);
|
||||
m_stopDecoding = false;
|
||||
m_isDecoding = true;
|
||||
}
|
||||
m_decoder->setParityMode(parity);
|
||||
m_decoder->serialDecode(baudRate);
|
||||
|
|
|
|||
|
|
@ -1,8 +1,9 @@
|
|||
#ifndef ISOBUFFER_H
|
||||
#define ISOBUFFER_H
|
||||
|
||||
// TODO: Make object macros constexprs or globals
|
||||
// TODO: Move headers used only in implementation to isobuffer.cpp
|
||||
#include <memory>
|
||||
|
||||
#include <QWidget>
|
||||
#include <QString>
|
||||
#include <QByteArray>
|
||||
|
|
@ -28,15 +29,13 @@ enum class UartParity : uint8_t;
|
|||
constexpr uint32_t CONSOLE_UPDATE_TIMER_PERIOD = ISO_PACKETS_PER_CTX * 4;
|
||||
|
||||
// TODO: Make private what should be private
|
||||
// TODO: Add m_ prefix to member variables
|
||||
// TODO: Change integer types to cstdint types
|
||||
class isoBuffer : public QWidget
|
||||
{
|
||||
Q_OBJECT
|
||||
public:
|
||||
// TODO: Add consoles as constructor arguments
|
||||
isoBuffer(QWidget* parent = 0, int bufferLen = 0, isoDriver* caller = 0, unsigned char channel_value = 0);
|
||||
// TODO?: Add a destructor
|
||||
~isoBuffer() = default;
|
||||
|
||||
// Basic buffer operations
|
||||
short bufferAt(int idx) const;
|
||||
|
|
@ -52,11 +51,13 @@ public:
|
|||
void writeBuffer_char(char* data, int len);
|
||||
void writeBuffer_short(short* data, int len);
|
||||
|
||||
// TODO: Change return value to unique_ptr
|
||||
short* readBuffer(double sampleWindow, int numSamples, bool singleBit, double delayOffset);
|
||||
std::unique_ptr<short[]> readBuffer(double sampleWindow, int numSamples, bool singleBit, double delayOffset);
|
||||
|
||||
// file I/O
|
||||
bool maybeOutputSampleToFile(double convertedSample);
|
||||
private:
|
||||
void outputSampleToFile(double averageSample);
|
||||
void maybeOutputSampleToFile(double convertedSample);
|
||||
public:
|
||||
double sampleConvert(short sample, int TOP, bool AC) const;
|
||||
short inverseSampleConvert(double voltageLevel, int TOP, bool AC) const;
|
||||
|
||||
|
|
@ -71,43 +72,41 @@ public:
|
|||
|
||||
// ---- MEMBER VARIABLES ----
|
||||
|
||||
// Presentantion?
|
||||
// NOTE: it seems like these are never initialized but they are used as though they were...
|
||||
// Presentation?
|
||||
// TODO: Add consoles as constructor arguments
|
||||
// NOTE: These are initialized in mainwindow.cpp
|
||||
QPlainTextEdit* m_console1;
|
||||
QPlainTextEdit* m_console2;
|
||||
unsigned char m_channel = 255;
|
||||
bool m_serialAutoScroll = true;
|
||||
|
||||
// Internal Storage
|
||||
std::unique_ptr<short[]> m_buffer;
|
||||
int m_back = 0;
|
||||
int m_insertedCount = 0;
|
||||
int m_bufferEnd;
|
||||
|
||||
// Conversion And Sampling
|
||||
double m_voltage_ref = 1.65;
|
||||
double m_frontendGain = (R4 / (R3 + R4));
|
||||
int m_samplesPerSecond;
|
||||
int m_sampleRate_bit;
|
||||
|
||||
// Internal Storage
|
||||
int m_back = 0;
|
||||
int m_insertedCount = 0;
|
||||
int m_bufferEnd;
|
||||
// TODO: Change buffer to be a unique_ptr
|
||||
short* m_buffer;
|
||||
short* m_readData = NULL;
|
||||
|
||||
// UARTS decoding
|
||||
uartStyleDecoder* m_decoder = NULL;
|
||||
// TODO: change this to keepDecoding
|
||||
bool m_stopDecoding = false;
|
||||
bool m_isDecoding = true;
|
||||
private:
|
||||
// File I/O
|
||||
bool m_fileIOEnabled = false;
|
||||
FILE* m_fptr = NULL;
|
||||
QFile* m_currentFile;
|
||||
int m_fileIO_maxIncrementedSampleValue;
|
||||
int m_fileIO_sampleCountPerWrite;
|
||||
int m_fileIO_sampleCount;
|
||||
qulonglong m_fileIO_max_file_size;
|
||||
double m_fileIO_sampleAccumulator;
|
||||
qulonglong m_fileIO_maxFileSize;
|
||||
qulonglong m_fileIO_numBytesWritten;
|
||||
unsigned int m_currentColumn = 0;
|
||||
|
||||
isoDriver* m_virtualParent;
|
||||
double m_average_sample_temp;
|
||||
signals:
|
||||
void fileIOinternalDisable();
|
||||
public slots:
|
||||
|
|
|
|||
|
|
@ -806,23 +806,23 @@ void isoDriver::frameActionGeneric(char CH1_mode, char CH2_mode) //0 for off, 1
|
|||
|
||||
|
||||
if (CH1_mode == 1){
|
||||
analogConvert(readData375_CH1, &CH1, 128, AC_CH1, 1);
|
||||
analogConvert(readData375_CH1.get(), &CH1, 128, AC_CH1, 1);
|
||||
xmin = (currentVmin < xmin) ? currentVmin : xmin;
|
||||
xmax = (currentVmax > xmax) ? currentVmax : xmax;
|
||||
broadcastStats(0);
|
||||
}
|
||||
if (CH1_mode == 2) digitalConvert(readData375_CH1, &CH1);
|
||||
if (CH1_mode == 2) digitalConvert(readData375_CH1.get(), &CH1);
|
||||
|
||||
if (CH2_mode == 1){
|
||||
analogConvert(readData375_CH2, &CH2, 128, AC_CH2, 2);
|
||||
analogConvert(readData375_CH2.get(), &CH2, 128, AC_CH2, 2);
|
||||
ymin = (currentVmin < ymin) ? currentVmin : ymin;
|
||||
ymax = (currentVmax > ymax) ? currentVmax : ymax;
|
||||
broadcastStats(1);
|
||||
}
|
||||
if (CH2_mode == 2) digitalConvert(readData375_CH2, &CH2);
|
||||
if (CH2_mode == 2) digitalConvert(readData375_CH2.get(), &CH2);
|
||||
|
||||
if(CH1_mode == -1) {
|
||||
analogConvert(readData750, &CH1, 128, AC_CH1, 1);
|
||||
analogConvert(readData750.get(), &CH1, 128, AC_CH1, 1);
|
||||
xmin = (currentVmin < xmin) ? currentVmin : xmin;
|
||||
xmax = (currentVmax > xmax) ? currentVmax : xmax;
|
||||
broadcastStats(0);
|
||||
|
|
@ -947,7 +947,7 @@ void isoDriver::multimeterAction(){
|
|||
readData375_CH1 = internalBuffer375_CH1->readBuffer(window,GRAPH_SAMPLES, false, delay + ((triggerEnabled&&!paused_multimeter) ? triggerDelay + window/2 : 0));
|
||||
|
||||
QVector<double> x(GRAPH_SAMPLES), CH1(GRAPH_SAMPLES);
|
||||
analogConvert(readData375_CH1, &CH1, 2048, 0, 1); //No AC coupling!
|
||||
analogConvert(readData375_CH1.get(), &CH1, 2048, 0, 1); //No AC coupling!
|
||||
|
||||
for (double i=0; i<GRAPH_SAMPLES; i++){
|
||||
x[i] = -(window*i)/((double)(GRAPH_SAMPLES-1)) - delay;
|
||||
|
|
@ -1337,7 +1337,7 @@ double isoDriver::meanVoltageLast(double seconds, unsigned char channel, int TOP
|
|||
break;
|
||||
}
|
||||
|
||||
short * tempBuffer = currentBuffer->readBuffer(seconds,1024, 0, 0);
|
||||
std::unique_ptr<short[]> tempBuffer = currentBuffer->readBuffer(seconds, 1024, 0, 0);
|
||||
double sum = 0;
|
||||
double temp;
|
||||
for(int i = 0; i<1024; i++){
|
||||
|
|
|
|||
|
|
@ -96,7 +96,9 @@ private:
|
|||
void frameActionGeneric(char CH1_mode, char CH2_mode);
|
||||
//Variables that are just pointers to other classes/vars
|
||||
QCustomPlot *axes;
|
||||
short *readData375_CH1, *readData375_CH2, *readData750;
|
||||
std::unique_ptr<short[]> readData375_CH1;
|
||||
std::unique_ptr<short[]> readData375_CH2;
|
||||
std::unique_ptr<short[]> readData750;
|
||||
float *readDataFile;
|
||||
char *isoTemp = NULL;
|
||||
short *isoTemp_short = NULL;
|
||||
|
|
|
|||
|
|
@ -83,7 +83,7 @@ void uartStyleDecoder::serialDecode(double baudRate)
|
|||
if(allZeroes){
|
||||
qDebug() << "Wire Disconnect detected!";
|
||||
wireDisconnected(parent->m_channel);
|
||||
parent->m_stopDecoding = true;
|
||||
parent->m_isDecoding = false;
|
||||
updateTimer->stop();
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue