-
Notifications
You must be signed in to change notification settings - Fork 8
OS Interface + Static Buffer Allocation #120
Conversation
…lass without duplicating code, and prevent dynamic heap allocation
Runtime verified by @gokuldharan yesterday |
@@ -140,6 +141,7 @@ namespace{ | |||
|
|||
|
|||
buffer::BufferMaster BufferMaster; | |||
os::OsInterfaceImpl osInterfaceImpl; |
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try changing portMAX_DELAY to the cmsis equivalent. Looks good, should be able to pull in after
Common/include/BufferBase.h
Outdated
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, portMAX_DELAY); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there's a cmsis version of portMAX_DELAY called osWaitForever or something like that. Could you try to use the cmsis one instead? Robert mentioned it recently and I think it's a good idea since cmsis is generic
…r doc changes Runtime verified + all tests passing
to allow direct testing of class without duplicating code, and prevent dynamic heap allocation