Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor Os::Queue into CMake selection #2895

Merged
merged 25 commits into from
Sep 30, 2024
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
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 .github/actions/spelling/expect.txt
Original file line number Diff line number Diff line change
Expand Up @@ -794,6 +794,7 @@
pri
printables
prioritization
PRIORITYQUEUE
prm
PRMDB
PRMDBIMPL
Expand Down Expand Up @@ -997,6 +998,7 @@
stddef
stdint
stest
Stl
STREQ
STREQUAL
strerror
Expand All @@ -1016,9 +1018,9 @@
subseconds
subtargets
subtopology
subtopologies

Check warning on line 1021 in .github/actions/spelling/expect.txt

View workflow job for this annotation

GitHub Actions / Spell checking

`subtopologies` is ignored by check spelling because another more general variant is also in expect. (ignored-expect-variant)
Subtopology

Check warning on line 1022 in .github/actions/spelling/expect.txt

View workflow job for this annotation

GitHub Actions / Spell checking

`Subtopology` is ignored by check spelling because another more general variant is also in expect. (ignored-expect-variant)
Subtopologies

Check warning on line 1023 in .github/actions/spelling/expect.txt

View workflow job for this annotation

GitHub Actions / Spell checking

`Subtopologies` is ignored by check spelling because another more general variant is also in expect. (ignored-expect-variant)
suppr
suseconds
SVCLOGFILE
Expand Down Expand Up @@ -1216,4 +1218,4 @@
xxxx
yacgen
zimri
zmq
zmq
2 changes: 1 addition & 1 deletion FppTest/state_machine/test/ut/SmTestTester.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ void SmTestTester::schedIn_OK() {
void SmTestTester ::
dispatchAll()
{
while (this->component.m_queue.getNumMsgs() > 0)
while (this->component.m_queue.getMessagesAvailable() > 0)
this->component.doDispatch();
}

Expand Down
4 changes: 2 additions & 2 deletions Fw/Comp/ActiveComponentBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ namespace Fw {
ActiveComponentExitSerializableBuffer exitBuff;
SerializeStatus stat = exitBuff.serialize(static_cast<I32>(ACTIVE_COMPONENT_EXIT));
FW_ASSERT(FW_SERIALIZE_OK == stat,static_cast<NATIVE_INT_TYPE>(stat));
(void)this->m_queue.send(exitBuff,0,Os::Queue::QUEUE_NONBLOCKING);
(void)this->m_queue.send(exitBuff,0,Os::Queue::BlockingType::NONBLOCKING);
DEBUG_PRINT("exit %s\n", this->getObjName());
}

Expand Down Expand Up @@ -137,7 +137,7 @@ namespace Fw {

ActiveComponentBase::MsgDispatchStatus ActiveComponentBase::dispatch() {
// Cooperative tasks should return rather than block when no messages are available
if (this->m_task.isCooperative() and m_queue.getNumMsgs() == 0) {
if (this->m_task.isCooperative() and m_queue.getMessagesAvailable() == 0) {
return MsgDispatchStatus::MSG_DISPATCH_EMPTY;
}
return this->doDispatch();
Expand Down
2 changes: 1 addition & 1 deletion Fw/Comp/QueuedComponentBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
}
#endif

Os::Queue::QueueStatus QueuedComponentBase::createQueue(NATIVE_INT_TYPE depth, NATIVE_INT_TYPE msgSize) {
Os::Queue::Queue::Status QueuedComponentBase::createQueue(FwSizeType depth, FwSizeType msgSize) {

Check notice

Code scanning / CodeQL

Use of basic integral type Note

depth uses the basic integral type unsigned long rather than a typedef with size and signedness.

Check notice

Code scanning / CodeQL

Use of basic integral type Note

msgSize uses the basic integral type unsigned long rather than a typedef with size and signedness.
Dismissed Show dismissed Hide dismissed

Os::QueueString queueName;
#if FW_OBJECT_NAMES == 1
Expand Down
2 changes: 1 addition & 1 deletion Fw/Comp/QueuedComponentBase.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ namespace Fw {
virtual ~QueuedComponentBase(); //!< Destructor
void init(NATIVE_INT_TYPE instance); //!< initialization function
Os::Queue m_queue; //!< queue object for active component
Os::Queue::QueueStatus createQueue(NATIVE_INT_TYPE depth, NATIVE_INT_TYPE msgSize);
Os::Queue::Status createQueue(FwSizeType depth, FwSizeType msgSize);
virtual MsgDispatchStatus doDispatch()=0; //!< method to dispatch a single message in the queue.
#if FW_OBJECT_TO_STRING == 1
virtual void toString(char* str, NATIVE_INT_TYPE size); //!< dump string representation of component
Expand Down
2 changes: 1 addition & 1 deletion Os/Baremetal/Queue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ Queue::QueueStatus Queue::receive(U8* buffer, NATIVE_INT_TYPE capacity, NATIVE_I
return bareReceiveBlock(handle, buffer, capacity, actualSize, priority);
}

NATIVE_INT_TYPE Queue::getNumMsgs() const {
NATIVE_INT_TYPE Queue::getMessagesAvailable() const {
//Check if the handle is null or check the underlying queue is null
if ((nullptr == reinterpret_cast<BareQueueHandle*>(this->m_handle)) ||
(!reinterpret_cast<BareQueueHandle*>(this->m_handle)->m_init)) {
Expand Down
28 changes: 5 additions & 23 deletions Os/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
add_fprime_subdirectory("${CMAKE_CURRENT_LIST_DIR}/Models")
add_fprime_subdirectory("${CMAKE_CURRENT_LIST_DIR}/Stub")
add_fprime_subdirectory("${CMAKE_CURRENT_LIST_DIR}/Posix")
add_fprime_subdirectory("${CMAKE_CURRENT_LIST_DIR}/Generic")

set(SOURCE_FILES
"${CMAKE_CURRENT_LIST_DIR}/test/ut/file/SyntheticFileSystem.cpp"
Expand All @@ -26,10 +27,6 @@ set(MOD_DEPS
# Basic source files used in every OSAL layer. Contains common code and helpers.
set(SOURCE_FILES
"${CMAKE_CURRENT_LIST_DIR}/IntervalTimerCommon.cpp"
# "${CMAKE_CURRENT_LIST_DIR}/TaskCommon.cpp"
"${CMAKE_CURRENT_LIST_DIR}/QueueCommon.cpp"
"${CMAKE_CURRENT_LIST_DIR}/IPCQueueCommon.cpp"
"${CMAKE_CURRENT_LIST_DIR}/SimpleQueueRegistry.cpp"
"${CMAKE_CURRENT_LIST_DIR}/MemCommon.cpp"
"${CMAKE_CURRENT_LIST_DIR}/ValidateFileCommon.cpp"
"${CMAKE_CURRENT_LIST_DIR}/ValidatedFile.cpp"
Expand All @@ -41,6 +38,8 @@ set(SOURCE_FILES
"${CMAKE_CURRENT_LIST_DIR}/Mutex.cpp"
"${CMAKE_CURRENT_LIST_DIR}/FileSystem.cpp"
"${CMAKE_CURRENT_LIST_DIR}/Directory.cpp"
"${CMAKE_CURRENT_LIST_DIR}/Condition.cpp"
"${CMAKE_CURRENT_LIST_DIR}/Queue.cpp"
)
# Check for default logger
if (NOT FPRIME_DISABLE_DEFAULT_LOGGER)
Expand All @@ -52,10 +51,6 @@ endif()
if (FPRIME_USE_POSIX)
list(APPEND SOURCE_FILES
"${CMAKE_CURRENT_LIST_DIR}/Console.cpp"
"${CMAKE_CURRENT_LIST_DIR}/Pthreads/Queue.cpp"
"${CMAKE_CURRENT_LIST_DIR}/Pthreads/BufferQueueCommon.cpp"
"${CMAKE_CURRENT_LIST_DIR}/Pthreads/PriorityBufferQueue.cpp"
"${CMAKE_CURRENT_LIST_DIR}/Pthreads/MaxHeap/MaxHeap.cpp"
"${CMAKE_CURRENT_LIST_DIR}/Linux/InterruptLock.cpp"
"${CMAKE_CURRENT_LIST_DIR}/Linux/WatchdogTimer.cpp"
"${CMAKE_CURRENT_LIST_DIR}/Posix/IntervalTimer.cpp"
Expand All @@ -64,14 +59,12 @@ endif()
# Darwin IPC queue implementation
if(${CMAKE_SYSTEM_NAME} STREQUAL "Darwin")
list(APPEND SOURCE_FILES
"${CMAKE_CURRENT_LIST_DIR}/MacOs/IPCQueueStub.cpp"
# "${CMAKE_CURRENT_LIST_DIR}/MacOs/IPCQueueStub.cpp"
"${CMAKE_CURRENT_LIST_DIR}/MacOs/SystemResources.cpp"
)
# Linux IPC queues implementation
elseif (${CMAKE_SYSTEM_NAME} STREQUAL "Linux")
list(APPEND SOURCE_FILES
"${CMAKE_CURRENT_LIST_DIR}/Posix/IPCQueue.cpp"
"${CMAKE_CURRENT_LIST_DIR}/Posix/LocklessQueue.cpp"
"${CMAKE_CURRENT_LIST_DIR}/Linux/SystemResources.cpp"
)
# Shared libraries need an -rt dependency for mq libs
Expand Down Expand Up @@ -103,11 +96,11 @@ require_fprime_implementation(Os/Directory)
require_fprime_implementation(Os/Mutex)
# require_fprime_implementation(Os/Task) # should be added in
# require_fprime_implementation(Os/Console) # should be added in
require_fprime_implementation(Os/Queue)


### UTS ### Note: 3 separate UTs registered here.
set(UT_SOURCE_FILES
"${CMAKE_CURRENT_LIST_DIR}/test/ut/OsQueueTest.cpp"
"${CMAKE_CURRENT_LIST_DIR}/test/ut/OsTestMain.cpp"
"${CMAKE_CURRENT_LIST_DIR}/test/ut/IntervalTimerTest.cpp"
"${CMAKE_CURRENT_LIST_DIR}/test/ut/OsValidateFileTest.cpp"
Expand All @@ -127,14 +120,3 @@ if (BUILD_TESTING)
endif()
endforeach()
endif()
# Second UT Pthreads
set(UT_SOURCE_FILES
"${CMAKE_CURRENT_LIST_DIR}/Pthreads/test/ut/BufferQueueTest.cpp"
)
register_fprime_ut("Os_pthreads")

# Third UT Pthreads MAX Heap
set(UT_SOURCE_FILES
"${CMAKE_CURRENT_LIST_DIR}/Pthreads/MaxHeap/test/ut/MaxHeapTest.cpp"
)
register_fprime_ut("Os_pthreads_max_heap")
34 changes: 34 additions & 0 deletions Os/Condition.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
#include "Os/Condition.hpp"
#include "Fw/Types/Assert.hpp"

namespace Os {
ConditionVariable::ConditionVariable() : m_delegate(*ConditionVariableInterface::getDelegate(m_handle_storage)) {}
Dismissed Show dismissed Hide dismissed
Dismissed Show dismissed Hide dismissed

ConditionVariable::~ConditionVariable() {
m_delegate.~ConditionVariableInterface();
}

void ConditionVariable::wait(Os::Mutex& mutex) {
FW_ASSERT(&this->m_delegate == reinterpret_cast<ConditionVariableInterface*>(&this->m_handle_storage[0]));
FW_ASSERT(this->m_lock == nullptr || this->m_lock == &mutex); // Confirm the same mutex used across waits
this->m_lock = &mutex;

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter mutex has not been checked.
// Attempt to lock the mutex and ensure that this was successful or the lock was already held
Mutex::Status lock_status = this->m_lock->take();
FW_ASSERT(lock_status == Mutex::Status::OP_OK || lock_status == Mutex::Status::ERROR_DEADLOCK);
this->m_delegate.wait(mutex);
}
void ConditionVariable::notify() {
FW_ASSERT(&this->m_delegate == reinterpret_cast<ConditionVariableInterface*>(&this->m_handle_storage[0]));
this->m_delegate.notify();
}
void ConditionVariable::notifyAll() {
FW_ASSERT(&this->m_delegate == reinterpret_cast<ConditionVariableInterface*>(&this->m_handle_storage[0]));
this->m_delegate.notifyAll();
}

ConditionVariableHandle* ConditionVariable::getHandle(){
FW_ASSERT(&this->m_delegate == reinterpret_cast<const ConditionVariableInterface*>(&this->m_handle_storage[0]));
return this->m_delegate.getHandle();
}

}
125 changes: 125 additions & 0 deletions Os/Condition.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
// ======================================================================
// \title Os/Condition.hpp
// \brief common function definitions for Os::ConditionVariables
// ======================================================================
#include "Os/Mutex.hpp"
#include "Os/Os.hpp"

#ifndef OS_CONDITION_HPP_
#define OS_CONDITION_HPP_

namespace Os {

//! \brief Condition variable handle parent
class ConditionVariableHandle {};

//! \brief interface for condition variables
//!
//! Condition variables allow a program to block on a condition while atomically releasing an Os::Mutex and atomically
//! reacquiring the mutex once the condition has been notified.
class ConditionVariableInterface {
public:
//! Default constructor
ConditionVariableInterface() = default;
Dismissed Show dismissed Hide dismissed
//! Default destructor
virtual ~ConditionVariableInterface() = default;

//! \brief copy constructor is forbidden
ConditionVariableInterface(const ConditionVariableInterface& other) = delete;

//! \brief assignment operator is forbidden
virtual ConditionVariableInterface& operator=(const ConditionVariableInterface& other) = delete;

//! \brief wait on a condition variable
//!
//! Wait on a condition variable. This function will atomically unlock the provided mutex and block on the condition
//! in one step. Blocking will occur until a future `notify` or `notifyAll` call is made to this variable on another
//! thread of execution.
//!
//! \param mutex: mutex to unlock as part of this operation
virtual void wait(Os::Mutex& mutex) = 0;

//! \brief notify a single waiter on this condition variable
//!
//! Notify a single waiter on this condition variable. It is not necessary to hold the mutex supplied by the waiters
//! and it is advantageous not to hold the lock to prevent immediate re-blocking.
virtual void notify() = 0;

//! \brief notify all waiters on this condition variable
//!
//! Notify all waiters on this condition variable. It is not necessary to hold the mutex supplied by the waiters
//! and it is advantageous not to hold the lock to prevent immediate re-blocking.
virtual void notifyAll() = 0;

//! \brief return the underlying condition variable handle (implementation specific).
//! \return internal task handle representation
virtual ConditionVariableHandle* getHandle() = 0;

//! \brief provide a pointer to a Mutex delegate object
static ConditionVariableInterface* getDelegate(ConditionVariableHandleStorage& aligned_new_memory);
};

//! \brief condition variable implementation
//!
//! Condition variables allow a program to block on a condition while atomically releasing an Os::Mutex and atomically
//! reacquiring the mutex once the condition has been notified.
class ConditionVariable final : public ConditionVariableInterface {
public:
//! \brief default constructor
ConditionVariable();

//! \brief default virtual destructor
~ConditionVariable() final;

//! \brief copy constructor is forbidden
ConditionVariable(const ConditionVariableInterface& other) = delete;

//! \brief copy constructor is forbidden
ConditionVariable(const ConditionVariableInterface* other) = delete;

//! \brief assignment operator is forbidden
ConditionVariableInterface& operator=(const ConditionVariableInterface& other) override = delete;

//! \brief wait on a condition variable
//!
//! Wait on a condition variable. This function will atomically unlock the provided mutex and block on the condition
//! in one step. Blocking will occur until a future `notify` or `notifyAll` call is made to this variable on another
//! thread of execution. This function delegates to the underlying implementation.
//!
//! \warning it is invalid to supply a mutex different from those supplied by others
//! \warning conditions *must* be rechecked after the condition variable unlocks
//! \note unlocked mutexes will be locked before waiting and will be relocked before this function returns
//!
//! \param mutex: mutex to unlock as part of this operation
void wait(Os::Mutex& mutex) override;

//! \brief notify a single waiter on this condition variable
//!
//! Notify a single waiter on this condition variable. It is not necessary to hold the mutex supplied by the waiters
//! and it is advantageous not to hold the lock to prevent immediate re-blocking. This function delegates to the
//! underlying implementation.
void notify() override;

//! \brief notify all waiters on this condition variable
//!
//! Notify all waiters on this condition variable. It is not necessary to hold the mutex supplied by the waiters
//! and it is advantageous not to hold the lock to prevent immediate re-blocking. This function delegates to the
//! underlying implementation.
void notifyAll() override;

//! \brief return the underlying condition variable handle (implementation specific). Delegates to implementation.
//! \return internal task handle representation
ConditionVariableHandle* getHandle() override;

private:
//! Pointer to mutex object previously used
Os::Mutex* m_lock = nullptr;

// This section is used to store the implementation-defined file handle. To Os::File and fprime, this type is
// opaque and thus normal allocation cannot be done. Instead, we allow the implementor to store then handle in
// the byte-array here and set `handle` to that address for storage.
alignas(FW_HANDLE_ALIGNMENT) ConditionVariableHandleStorage m_handle_storage; //!< Storage for aligned FileHandle data
ConditionVariableInterface& m_delegate; //!< Delegate for the real implementation
};
}
#endif //OS_CONDITION_HPP_
2 changes: 0 additions & 2 deletions Os/Console.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
#include <Fw/Types/Assert.hpp>

namespace Os {
Console* Console::s_singleton;

Console::Console() : ConsoleInterface(), Fw::Logger(), m_handle_storage(), m_delegate(*ConsoleInterface::getDelegate(m_handle_storage)) {}

Console::~Console() {
Expand Down
1 change: 0 additions & 1 deletion Os/Console.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,6 @@ namespace Os {
static Console& getSingleton();

private:
static Console* s_singleton;
// This section is used to store the implementation-defined console handle. To Os::Console and fprime, this type
// is opaque and thus normal allocation cannot be done. Instead, we allow the implementor to store then handle
// in the byte-array here and set `handle` to that address for storage.
Expand Down
6 changes: 3 additions & 3 deletions Os/Delegate.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,11 @@ namespace Delegate {
//! \tparam Implementation: implementation class of the delegate (e.g. PosixTask)
//! \param aligned_new_memory: memory to be filled via placement new call
//! \return pointer to implementation result of placement new
template <class Interface, class Implementation>
inline Interface* makeDelegate(HandleStorage& aligned_new_memory) {
template <class Interface, class Implementation, class StorageType=HandleStorage>
inline Interface* makeDelegate(StorageType& aligned_new_memory) {
// Ensure prerequisites before performing placement new
static_assert(std::is_base_of<Interface, Implementation>::value, "Implementation must derive from Interface");
static_assert(sizeof(Implementation) <= FW_HANDLE_MAX_SIZE, "Handle size not large enough");
static_assert(sizeof(Implementation) <= sizeof(StorageType), "Handle size not large enough");
static_assert((FW_HANDLE_ALIGNMENT % alignof(Implementation)) == 0, "Handle alignment invalid");
// Placement new the object and ensure non-null result
Implementation* interface = new (aligned_new_memory) Implementation;
Expand Down
2 changes: 1 addition & 1 deletion Os/Directory.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class DirectoryInterface {
virtual DirectoryHandle* getHandle() = 0;

//! \brief provide a pointer to a Directory delegate object
static DirectoryInterface* getDelegate(HandleStorage& aligned_new_memory);
static DirectoryInterface* getDelegate(DirectoryHandleStorage& aligned_new_memory);


// -----------------------------------------------------------------
Expand Down
2 changes: 0 additions & 2 deletions Os/FileSystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
#include <Os/FileSystem.hpp>

namespace Os {
FileSystem* FileSystem::s_singleton;


FileSystem::FileSystem() : m_handle_storage(), m_delegate(*FileSystemInterface::getDelegate(m_handle_storage)) {
FW_ASSERT(&this->m_delegate == reinterpret_cast<FileSystemInterface*>(&this->m_handle_storage[0]));
Expand Down
4 changes: 1 addition & 3 deletions Os/FileSystem.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ class FileSystemInterface {
virtual FileSystemHandle* getHandle() = 0;

//! \brief provide a pointer to a FileSystem delegate object
static FileSystemInterface* getDelegate(HandleStorage& aligned_new_memory);
static FileSystemInterface* getDelegate(FileSystemHandleStorage& aligned_new_memory);


// ------------------------------------------------------------------
Expand Down Expand Up @@ -380,8 +380,6 @@ class FileSystem final : public FileSystemInterface {

alignas(FW_HANDLE_ALIGNMENT) FileSystemHandleStorage m_handle_storage; //!< FileSystem handle storage
FileSystemInterface& m_delegate;

static FileSystem* s_singleton;
};


Expand Down
Loading
Loading