Skip to content
This repository has been archived by the owner on Sep 27, 2021. It is now read-only.

OS Interface + Static Buffer Allocation #120

Merged
merged 2 commits into from
Nov 10, 2018
Merged

OS Interface + Static Buffer Allocation #120

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ab5e88e
OS Interface + Static Buffer Allocation: to allow direct testing of c…
gokuldharan Nov 6, 2018
d4e879a
Remove some unused includes, change os delays to CMSIS versions. Mino…
tygamvrelis Nov 10, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion Common/app/freertos.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
#include "imu_helper.h"
#include "rx_helper.h"
#include "tx_helper.h"
#include "OsInterfaceImpl.h"
/* USER CODE END Includes */

/* Variables -----------------------------------------------------------------*/
Expand Down Expand Up @@ -140,6 +141,7 @@ namespace{


buffer::BufferMaster BufferMaster;
os::OsInterfaceImpl osInterfaceImpl;
Copy link
Member

Choose a reason for hiding this comment

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

It seems to me that we should only need one osInterfaceImpl in all of our code. However, the memory footprint of an object with only functions is fairly small so it should be ok (relevant repl: https://repl.it/repls/InfamousMatureApplescript)


bool setupIsDone = false;
static volatile uint32_t error;
Expand Down Expand Up @@ -208,7 +210,7 @@ void MX_FREERTOS_Init(void) {
osMutexStaticDef(DATABUFFER, &DATABUFFERControlBlock);
DATABUFFERHandle = osMutexCreate(osMutex(DATABUFFER));
/* USER CODE BEGIN Initialize Data Buffer */
BufferMaster.set_lock(DATABUFFERHandle);
BufferMaster.setup_buffers(DATABUFFERHandle, &osInterfaceImpl);
/* USER CODE END Initialize Data Buffer */
/* USER CODE BEGIN RTOS_MUTEX */
/* add mutexes, ... */
Expand Down
130 changes: 30 additions & 100 deletions Common/include/BufferBase.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,64 +9,55 @@
*****************************************************************************
*/

// NOTE: defgroup used above since there is no .cpp file associated with this class.

#ifndef __BUFFER_BASE_H__
#define __BUFFER_BASE_H__
#ifndef BUFFER_BASE_H
#define BUFFER_BASE_H

/********************************** Includes **********************************/
#include <cstdio>
#include <memory>
#include "UART_Handler.h"
#include "MPU6050.h"
#include "PeripheralInstances.h"

#if defined(THREADED)
#include "cmsis_os.h"
#endif


namespace buffer {


/*********************************** Buffer ***********************************/
/**
* @class Generic templated thread-safe buffer class
* @class BufferBase Generic templated thread-safe buffer class
*/

#if defined(THREADED)
template <class T>
class BufferBase
{
public:
BufferBase()
BufferBase() {}
~BufferBase() {}
void set_osInterface(os::OsInterface *osInterface)
{
m_data_buf = std::unique_ptr<T>(new T);
m_read = -1;
m_osInterfacePtr = osInterface;
}
~BufferBase() {};
void set_lock(osMutexId lock)
{
m_lock = lock;
}
void write(const T &item)
{
xSemaphoreTake(m_lock, portMAX_DELAY);
*m_data_buf = item;
m_osInterfacePtr->OS_xSemaphoreTake(m_lock, osWaitForever);
m_databuf = item;
m_read = 0;
xSemaphoreGive(m_lock);
m_osInterfacePtr->OS_xSemaphoreGive(m_lock);
}
T read()
{
xSemaphoreTake(m_lock, portMAX_DELAY);
m_osInterfacePtr->OS_xSemaphoreTake(m_lock, osWaitForever);
m_read++;
xSemaphoreGive(m_lock);
return *m_data_buf;
m_osInterfacePtr->OS_xSemaphoreGive(m_lock);
return m_databuf;
}
void reset()
{
xSemaphoreTake(m_lock, portMAX_DELAY);
m_osInterfacePtr->OS_xSemaphoreTake(m_lock, osWaitForever);
m_read = -1;
xSemaphoreGive(m_lock);
m_osInterfacePtr->OS_xSemaphoreGive(m_lock);
}
int8_t num_reads()
{
Expand All @@ -75,34 +66,39 @@ class BufferBase

// Defining methods here in the declaration for ease of use as a templated class
private:
std::unique_ptr<T> m_data_buf;
T m_databuf;
//int indicates whether data has been read, -1 if data not written yet
int8_t m_read;
int8_t m_read = -1;
osMutexId m_lock = nullptr;
os::OsInterface* m_osInterfacePtr = nullptr;
};


/**
* @class Easily extendable thread-safe master class that holds all relevant buffers
* @class BufferMaster Easily extendible thread-safe master class that holds
* all relevant buffers
*/

class BufferMaster
{
public:
BufferMaster() {}
BufferMaster() {}
~BufferMaster() {}
void set_lock(osMutexId lock)
void setup_buffers(osMutexId lock, os::OsInterface *osInterface)
{
IMUBuffer.set_lock(lock);
IMUBuffer.set_osInterface(osInterface);
for(int i = 0; i < periph::NUM_MOTORS; ++i)
{
MotorBufferArray[i].set_lock(lock);
MotorBufferArray[i].set_osInterface(osInterface);
}
m_lock = lock;
m_osInterfacePtr = osInterface;
}
bool all_data_ready()
{
xSemaphoreTake(m_lock, portMAX_DELAY);
m_osInterfacePtr->OS_xSemaphoreTake(m_lock, osWaitForever);
bool ready = (IMUBuffer.num_reads() == 0);

if(ready)
Expand All @@ -112,88 +108,22 @@ class BufferMaster
ready = (ready && MotorBufferArray[i].num_reads() == 0);
}
}
xSemaphoreGive(m_lock);
m_osInterfacePtr->OS_xSemaphoreGive(m_lock);
return ready;
}
BufferBase<imu::IMUStruct_t> IMUBuffer;
BufferBase<MotorData_t> MotorBufferArray[periph::NUM_MOTORS];
// Add buffer items here as necessary
private:
osMutexId m_lock = nullptr;
os::OsInterface* m_osInterfacePtr = nullptr;
};

#else
/**
* @class Generic templated buffer class (not thread-safe)
*/
template <class T>
class BufferBase
{
public:
BufferBase()
{
m_data_buf = std::unique_ptr<T>(new T);
m_read = -1;
}
~BufferBase() {};
void write(const T &item)
{
*m_data_buf = item;
m_read = 0;
}
T read()
{
m_read++;
return *m_data_buf;
}
void reset()
{
m_read = -1;
}
int8_t num_reads()
{
return m_read;
}

// Defining methods here in the declaration for ease of use as a templated class
private:
std::unique_ptr<T> m_data_buf;
int8_t m_read;
};


/**
* @class Easily extendable master class that holds all relevant buffers (not thread-safe)
*/

class BufferMaster
{
public:
BufferMaster() {}
~BufferMaster() {}
bool all_data_ready()
{
bool ready = (IMUBuffer.num_reads() == 0);

if(ready)
{
for(int i = 0; i < periph::NUM_MOTORS; ++i)
{
ready = (ready && MotorBufferArray[i].num_reads() == 0);
}
}
return ready;
}
BufferBase<imu::IMUStruct_t> IMUBuffer;
BufferBase<MotorData_t> MotorBufferArray[periph::NUM_MOTORS];
// Add buffer items here as necessary
};
#endif
} // end namespace buffer

/**
* @}
*/
/* end - BufferBase */

#endif /* __BUFFER_BASE_H__ */
#endif /* BUFFER_BASE_H */
28 changes: 25 additions & 3 deletions Common/test/Buffer_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@
/********************************* Includes **********************************/
#include "UART_Handler.h"
#include "PeripheralInstances.h"

#undef THREADED
#include "MockOsInterface.h"
#include "BufferBase.h"

#include <gtest/gtest.h>
Expand All @@ -29,26 +28,37 @@ using ::testing::SetArgPointee;
using ::testing::Return;
using ::testing::_;

using mocks::MockOsInterface;
using namespace buffer;




/******************************** File-local *********************************/
namespace{
// Variables
// ----------------------------------------------------------------------------
MockOsInterface os;
osMutexId mutex = nullptr;

// Functions
// ----------------------------------------------------------------------------
TEST(BufferTests, CanInitializeBuffer){
BufferBase<int> intBuffer;
intBuffer.set_lock(mutex);
intBuffer.set_osInterface(&os);
}

TEST(BufferTests, CanWriteToBuffer){
BufferBase<int> intBuffer;
intBuffer.set_lock(mutex);
intBuffer.set_osInterface(&os);
intBuffer.write(10);
}

TEST(BufferTests, NumReadsCheck){
BufferBase<int> intBuffer;
intBuffer.set_lock(mutex);
intBuffer.set_osInterface(&os);

ASSERT_EQ(intBuffer.num_reads() , -1);
intBuffer.write(10);
Expand All @@ -57,6 +67,9 @@ TEST(BufferTests, NumReadsCheck){

TEST(BufferTests, CanReadFromBuffer){
BufferBase<int> intBuffer;
intBuffer.set_lock(mutex);
intBuffer.set_osInterface(&os);

intBuffer.write(10);

int result = intBuffer.read();
Expand All @@ -70,6 +83,9 @@ TEST(BufferTests, CanReadFromBuffer){

TEST(BufferTests, CanResetBuffer){
BufferBase<int> intBuffer;
intBuffer.set_lock(mutex);
intBuffer.set_osInterface(&os);

intBuffer.write(10);

ASSERT_EQ(intBuffer.num_reads() , 0);
Expand All @@ -79,17 +95,20 @@ TEST(BufferTests, CanResetBuffer){

TEST(BufferTests, CanInitializeBufferMaster){
BufferMaster bufferMaster;
bufferMaster.setup_buffers(mutex,&os);
}

TEST(BufferTests, CanWriteToIMUBuffer){
BufferMaster bufferMaster;
bufferMaster.setup_buffers(mutex,&os);
imu::IMUStruct_t IMUdata;

bufferMaster.IMUBuffer.write(IMUdata);
}

TEST(BufferTests, CanReadFromIMUBuffer){
BufferMaster bufferMaster;
bufferMaster.setup_buffers(mutex,&os);
imu::IMUStruct_t IMUdata;
IMUdata.x_Accel = 1.0;
IMUdata.x_Gyro = 2.0;
Expand All @@ -112,6 +131,7 @@ TEST(BufferTests, CanReadFromIMUBuffer){
TEST(BufferTests, CanWriteToMotorBuffer){
//TODO: Use periph::NUM_MOTORS without defining THREADED
BufferMaster bufferMaster;
bufferMaster.setup_buffers(mutex,&os);
MotorData_t motorData[periph::NUM_MOTORS];

for(int i = 0; i < periph::NUM_MOTORS; ++i)
Expand All @@ -122,6 +142,7 @@ TEST(BufferTests, CanWriteToMotorBuffer){

TEST(BufferTests, CanReadMotorDataBuffer){
BufferMaster bufferMaster;
bufferMaster.setup_buffers(mutex,&os);
MotorData_t motorData[periph::NUM_MOTORS];
MotorData_t readMotorData[periph::NUM_MOTORS];

Expand All @@ -136,6 +157,7 @@ TEST(BufferTests, CanReadMotorDataBuffer){

TEST(BufferTests, CanConfirmAllDataReady){
BufferMaster bufferMaster;
bufferMaster.setup_buffers(mutex,&os);
MotorData_t motorData[periph::NUM_MOTORS];
imu::IMUStruct_t IMUdata;

Expand Down