Skip to content

Update additional classes in Os to offer the alias option #5955

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.

Sign up for GitHub

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

Closed
bevinduckett wants to merge 13 commits into from
Closed

Update additional classes in Os to offer the alias option #5955

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
37c6d03
checkpoint: Added alias for File, FileSystem, Directory, Cpu, Memory,…
Aug 17, 2026
d208521
Added File & Directory headers
Aug 19, 2026
af3538a
Revert "Merge branch 'devel' into multi-os-alias"
Aug 19, 2026
8b7b9ab
checkpoint
Aug 24, 2026
1deac3b
Updated Os::File & Os::Directory to fix build & UT
Sep 3, 2026
25f53fd
Fixed singleton pattern in Os Cpu, Memory, and FileSystem classes
Sep 12, 2026
bc98748
Removed support for Os::Mutex alias
Sep 12, 2026
4286915
Cleanup
Sep 12, 2026
4eb26cd
Merge branch 'devel' into multi-os-alias
Sep 12, 2026
d739adc
Reconcile with devel: restore files reverted by the stale devel-merge…
Sep 12, 2026
14a991f
Merge branch 'devel' into multi-os-alias
bevinduckett Sep 23, 2026
59ddb89
Fixes for PR comments
Sep 24, 2026
459f5dd
Merge branch 'devel' into multi-os-alias
Sep 25, 2026
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
120 changes: 115 additions & 5 deletions Os/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,61 @@ register_fprime_module(
Fw_Types
)

add_named_os_module(Console Fw_Logger)
add_named_os_module("File;FileSystem;Directory" Utils_Hash Fw_DataStructures)
# Console: Console.hpp is a thin public alias header (no .cpp); the interface and
# link-time delegate live in ConsoleInterface.hpp and DelegateConsole.{hpp,cpp}.
# Registered explicitly because add_named_os_module assumes a matching
# <name>.cpp per header.
register_fprime_module(
"Os_Console"
REQUIRES_IMPLEMENTATIONS
"Os_Console"
SOURCES
"${CMAKE_CURRENT_LIST_DIR}/ConsoleInterface.cpp"

@lestarch-autobot lestarch-autobot Sep 25, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Design] must fix design-code-mismatch: ConsoleInterface.cpp and ConditionVariableInterface.cpp (line 207) are registered as sources but neither file exists at this head, while their definitions (ConsoleInterface::writeMessage(const Fw::ConstStringBase&), ConditionVariableInterface::wait) were removed from DelegateConsole.cpp / DelegateConditionVariable.cpp. The intended design — interface convenience methods live in their own <Name>Interface.cpp TU like CpuInterface.cpp — is right, but the files were not added, so CMake generation fails (every CI build at 59ddb89 is red) and even with the SOURCES lines dropped the two methods would be undefined at link time (DelegateConditionVariable::wait now calls the base). Please add both files with the moved definitions.

@lestarch-autobot lestarch-autobot Sep 25, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Maintainability] Concur — also in scope for maintainability (structural hazard): the move out of DelegateConsole.cpp/DelegateConditionVariable.cpp (my prior thread) is the right shape, but with neither <Name>Interface.cpp committed the build no longer has a definition for either method; severity from my scope: must fix.

@lestarch-autobot lestarch-autobot Sep 25, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Correctness] Concur — also in scope for functional correctness: ConditionVariableInterface::wait and ConsoleInterface::writeMessage(const Fw::ConstStringBase&) are declared and called (DelegateConditionVariable::wait, Fw::Logger::log) but have no definition anywhere in the tree, so the build cannot link even after CMake generation is repaired; severity from my scope: must fix.

@lestarch-autobot lestarch-autobot Sep 25, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[C++ Design] must fix CPP-26 (F Prime style): declared members with no definition in the tree; the build no longer configures.

ConsoleInterface.cpp (here) and ConditionVariableInterface.cpp (line 207) are listed as SOURCES but neither file exists at this head, so CMake fails to configure Os_Console/Os_Condition. The definitions they presumably carry — ConsoleInterface::writeMessage(const Fw::ConstStringBase&) and ConditionVariableInterface::wait(Os::Mutex&) — were removed from DelegateConsole.cpp/DelegateConditionVariable.cpp in 59ddb89 and now exist nowhere (DelegateConditionVariable::wait calls the undefined one at Os/DelegateConditionVariable.cpp:28). Add the two files to the commit (or restore the definitions in the delegate translation units).

@lestarch-autobot lestarch-autobot Sep 25, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Summary] Duplicate — consolidated into #5955 (comment) (same site, Os/CMakeLists.txt:147; same issue — missing ConsoleInterface.cpp/ConditionVariableInterface.cpp).

"${CMAKE_CURRENT_LIST_DIR}/DelegateConsole.cpp"
HEADERS
"${CMAKE_CURRENT_LIST_DIR}/Console.hpp"
"${CMAKE_CURRENT_LIST_DIR}/ConsoleInterface.hpp"
"${CMAKE_CURRENT_LIST_DIR}/DelegateConsole.hpp"
DEPENDS
Fw_Time
Fw_Types
Fw_Logger
)
fprime_target_dependencies(Os PUBLIC "Os_Console")

# File: File.hpp is a thin public alias header; the interface and link-time
# delegate live in FileInterface.hpp and DelegateFile.{hpp,cpp}. FileSystem
# and Directory follow the same pattern. Registered explicitly because
# add_named_os_module assumes a matching <name>.cpp per header.
register_fprime_module(
"Os_File"
REQUIRES_IMPLEMENTATIONS
"Os_File"
SOURCES
"${CMAKE_CURRENT_LIST_DIR}/FileInterface.cpp"
"${CMAKE_CURRENT_LIST_DIR}/DelegateFile.cpp"
"${CMAKE_CURRENT_LIST_DIR}/DelegateFileSystem.cpp"
"${CMAKE_CURRENT_LIST_DIR}/FileSystemInterface.cpp"
"${CMAKE_CURRENT_LIST_DIR}/DirectoryInterface.cpp"
"${CMAKE_CURRENT_LIST_DIR}/DelegateDirectory.cpp"
HEADERS
"${CMAKE_CURRENT_LIST_DIR}/File.hpp"
"${CMAKE_CURRENT_LIST_DIR}/FileInterface.hpp"
"${CMAKE_CURRENT_LIST_DIR}/DelegateFile.hpp"
"${CMAKE_CURRENT_LIST_DIR}/FileSystem.hpp"
"${CMAKE_CURRENT_LIST_DIR}/FileSystemInterface.hpp"
"${CMAKE_CURRENT_LIST_DIR}/DelegateFileSystem.hpp"
"${CMAKE_CURRENT_LIST_DIR}/Directory.hpp"
"${CMAKE_CURRENT_LIST_DIR}/DirectoryInterface.hpp"
"${CMAKE_CURRENT_LIST_DIR}/DelegateDirectory.hpp"
DEPENDS
Fw_Time
Fw_Types
Utils_Hash
Fw_DataStructures
)
fprime_target_dependencies(Os PUBLIC "Os_File")

add_named_os_module(Task ${CMAKE_THREAD_LIBS_INIT})
# Mutex: Mutex.hpp is a thin public alias header (no Mutex.cpp); the interface and
# link-time delegate live in MutexInterface.hpp / DelegateMutex.{hpp,cpp}. Registered
Comment on lines 194 to 195

@lestarch-autobot lestarch-autobot Sep 26, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Maintainability] could fix This seven-line comment restates the file list of the register_fprime_module block directly beneath it (MutexInterface.hpp, DelegateMutex.{hpp,cpp}, Condition.hpp, ...), so any rename/move of those files must be made twice or the comment silently goes stale; the sibling Console/Cpu/Memory blocks keep their comments to the one non-obvious fact (why add_named_os_module is not used).

Suggested change
# Mutex: Mutex.hpp is a thin public alias header (no Mutex.cpp); the interface and
# link-time delegate live in MutexInterface.hpp / DelegateMutex.{hpp,cpp}. Registered
# Mutex + ConditionVariable: Mutex.hpp / Condition.hpp are thin alias headers (no matching .cpp), so
# add_named_os_module does not fit; both stay in one Os_Mutex module because the Stub impl and
# REQUIRES_IMPLEMENTATIONS references depend on that name.

@lestarch-autobot lestarch-autobot Sep 26, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Maintainability] Note: GitHub only allows anchoring on the diff hunk (lines 194-195); the suggestion is meant to replace the whole comment, lines 194-200.

Expand Down Expand Up @@ -164,10 +217,67 @@ register_fprime_module(
Fw_Types
)
fprime_target_dependencies(Os PUBLIC "Os_Mutex")
add_named_os_module(CountingSemaphore)
# CountingSemaphore: CountingSemaphore.hpp is a thin public alias header (no
# .cpp); the interface and link-time delegate live in
# CountingSemaphoreInterface.hpp and DelegateCountingSemaphore.{hpp,cpp}.
# Registered explicitly because add_named_os_module assumes a matching
# <name>.cpp per header.
register_fprime_module(
"Os_CountingSemaphore"
REQUIRES_IMPLEMENTATIONS
"Os_CountingSemaphore"
SOURCES
"${CMAKE_CURRENT_LIST_DIR}/DelegateCountingSemaphore.cpp"
HEADERS
"${CMAKE_CURRENT_LIST_DIR}/CountingSemaphore.hpp"
"${CMAKE_CURRENT_LIST_DIR}/CountingSemaphoreInterface.hpp"
"${CMAKE_CURRENT_LIST_DIR}/DelegateCountingSemaphore.hpp"
DEPENDS
Fw_Time
Fw_Types
)
fprime_target_dependencies(Os PUBLIC "Os_CountingSemaphore")
add_named_os_module(Queue)
add_named_os_module(Cpu)
add_named_os_module(Memory)
# Cpu: Cpu.hpp is a thin public alias header (no .cpp); the interface and
# link-time delegate live in CpuInterface.hpp and DelegateCpu.{hpp,cpp}.
# Registered explicitly because add_named_os_module assumes a matching
# <name>.cpp per header.
register_fprime_module(
"Os_Cpu"
REQUIRES_IMPLEMENTATIONS
"Os_Cpu"
SOURCES
"${CMAKE_CURRENT_LIST_DIR}/DelegateCpu.cpp"
"${CMAKE_CURRENT_LIST_DIR}/CpuInterface.cpp"
HEADERS
"${CMAKE_CURRENT_LIST_DIR}/Cpu.hpp"
"${CMAKE_CURRENT_LIST_DIR}/CpuInterface.hpp"
"${CMAKE_CURRENT_LIST_DIR}/DelegateCpu.hpp"
DEPENDS
Fw_Time
Fw_Types
)
fprime_target_dependencies(Os PUBLIC "Os_Cpu")
# Memory: Memory.hpp is a thin public alias header (no .cpp); the interface and
# link-time delegate live in MemoryInterface.hpp and DelegateMemory.{hpp,cpp}.
# Registered explicitly because add_named_os_module assumes a matching
# <name>.cpp per header.
register_fprime_module(
"Os_Memory"
REQUIRES_IMPLEMENTATIONS
"Os_Memory"
SOURCES
"${CMAKE_CURRENT_LIST_DIR}/DelegateMemory.cpp"
"${CMAKE_CURRENT_LIST_DIR}/MemoryInterface.cpp"
HEADERS
"${CMAKE_CURRENT_LIST_DIR}/Memory.hpp"
"${CMAKE_CURRENT_LIST_DIR}/MemoryInterface.hpp"
"${CMAKE_CURRENT_LIST_DIR}/DelegateMemory.hpp"
DEPENDS
Fw_Time
Fw_Types
)
fprime_target_dependencies(Os PUBLIC "Os_Memory")
# RawTime: RawTime.hpp is a thin public alias header (no .cpp); the interface and
# link-time delegate live in DelegateRawTime.{hpp,cpp}. Registered explicitly
# because add_named_os_module assumes a matching <name>.cpp per header.
Expand Down
72 changes: 0 additions & 72 deletions Os/Console.cpp
View file
Open in desktop

This file was deleted.

160 changes: 31 additions & 129 deletions Os/Console.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,139 +1,41 @@
// ======================================================================
// \title Os/Console.hpp
// \brief common function definitions for Os::Console
// \brief public Os::Console interface and alias
//
// This header aggregates all definitions needed to use Os::Console:
// the interface, the configured alias, and the concrete delegate type.
//
// WARNING — include order is load-bearing. Do not reorder.
//
// The dependency constraints are:
//
// 1. config/OsDelegateConsole.hpp (CFG) defines the Os::Console type alias by
// forward-declaring a link-time delegate (e.g. DelegateConsole) or directly
// aliasing a concrete implementation (e.g. Va416x0::GdsConsole).
// Must not include Os OSAL headers (they aren't yet defined).
// Should only be included in Os/ConsoleInterface.hpp.
//
// 2. Os/ConsoleInterface.hpp (IF) includes CFG first (so the Os::Console alias
// is available), then defines ConsoleHandle and ConsoleInterface.
//
// 3. OS_CONSOLE_HEADER (IMPL) is defined by CFG and points to the concrete
// implementation header. If using delegation, this points to
// Os/DelegateConsole.hpp. If using compile-time selection, it points
// directly to a platform-specific implementation (e.g. GdsConsole.hpp).
//
// ConsoleInterface.hpp must precede OS_CONSOLE_HEADER here,
// and CFG must never include either of them (that would form a cycle).
// ======================================================================
#ifndef Os_Console_hpp_
#define Os_Console_hpp_

#include <Fw/FPrimeBasicTypes.hpp>
#include <Fw/Logger/Logger.hpp>
#include <Os/Console.hpp>
#include <Os/Os.hpp>
#include "Os/ConsoleInterface.hpp"

namespace Os {
//! \brief Base class for storing implementation specific handle information
struct ConsoleHandle {};

// \brief Interface defining the properties of the console
class ConsoleInterface {
public:
//! \brief Default constructor
ConsoleInterface() = default;

//! \brief Default destructor
virtual ~ConsoleInterface() = default;

//! \brief write message to console
//!
//! Write a message to the console with a bounded size.
//!
//! \param message: raw message to write
//! \param size: size of the message to write to the console
virtual void writeMessage(const CHAR* message, const FwSizeType size) = 0;

//! \brief returns the raw console handle
//!
//! Gets the raw console handle from the implementation. Note: users must include the implementation specific
//! header to make any real use of this handle. Otherwise it will be as an opaque type.
//!
//! \return raw console handle
//!
virtual ConsoleHandle* getHandle() = 0;

//! \brief provide a pointer to a console delegate object
//!
//! This function must return a pointer to a `ConsoleInterface` object that contains the real implementation of
//! the console functions as defined by the implementor. This function must do several things to be considered
//! correctly implemented:
//!
//! 1. Assert that their implementation fits within FW_HANDLE_MAX_SIZE.
//! e.g. `static_assert(sizeof(PosixFileImplementation) <= sizeof Os::File::m_handle_storage,
//! "FW_HANDLE_MAX_SIZE too small");`
//! 2. Assert that their implementation aligns within FW_HANDLE_ALIGNMENT.
//! e.g. `static_assert((FW_HANDLE_ALIGNMENT % alignof(PosixFileImplementation)) == 0, "Bad handle alignment");`
//! 3. If to_copy is null, placement new their implementation into `aligned_placement_new_memory`
//! e.g. `FileInterface* interface = new (aligned_placement_new_memory) PosixFileImplementation;`
//! 4. If to_copy is non-null, placement new using copy constructor their implementation into
//! `aligned_placement_new_memory`
//! e.g. `FileInterface* interface = new (aligned_placement_new_memory) PosixFileImplementation(*to_copy);`
//! 5. Return the result of the placement new
//! e.g. `return interface;`
//!
//! \return result of placement new, must be equivalent to `aligned_placement_new_memory`
//!
static ConsoleInterface* getDelegate(ConsoleHandleStorage& aligned_placement_new_memory,
const ConsoleInterface* to_copy = nullptr);
};

class Console : public ConsoleInterface, public Fw::Logger {
public:
//! \brief Default constructor
Console();

//! \brief Default destructor
~Console();

//! \brief copy constructor that copies the internal representation
Console(const Console& other);

//! \brief assignment operator that copies the internal representation
Console& operator=(const Console& other);

//! \brief write message to console
//!
//! Write a message to the console with a bounded size. This will delegate to the implementation defined write
//! method.
//!
//! \param message: raw message to write
//! \param size: size of the message to write to the console
void writeMessage(const CHAR* message, const FwSizeType size) override;

//! \brief write message to console
//!
//! Write a message to the console as stored as a ConstStringBase type
//!
//! \param message: raw message to write (ConstStringBase)
void writeMessage(const Fw::ConstStringBase& message) override;

//! \brief returns the raw console handle
//!
//! Gets the raw console handle from the implementation. Note: users must include the implementation specific
//! header to make any real use of this handle. Otherwise it will be as an opaque type.
//!
//! \return raw console handle
//!
ConsoleHandle* getHandle() override;

//! \brief write message to console
//!
//! Write a message to the console as stored as a ConstStringBase type
//!
//! \param message: raw message to write (ConstStringBase)
static void write(const Fw::ConstStringBase& message);

//! \brief write message to the global console
//!
//! Write a message to the console with a bounded size. This will delegate to the global singleton
//! implementation.
//!
//! \param message: raw message to write
//! \param size: size of the message to write to the console
static void write(const CHAR* message, const FwSizeType size);

//! \brief initialize singleton
static void init();

//! \brief get a reference to singleton
//! \return reference to singleton
static Console& getSingleton();
// Validate that OS_CONSOLE_HEADER was defined by config/OsDelegateConsole.hpp
#ifndef OS_CONSOLE_HEADER
#error "OS_CONSOLE_HEADER must be defined in config/OsDelegateConsole.hpp"
#endif

private:
// 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.
alignas(FW_HANDLE_ALIGNMENT) ConsoleHandleStorage m_handle_storage; // Storage for the delegate
ConsoleInterface& m_delegate; //!< Delegate for the real implementation
};
} // namespace Os
#include OS_CONSOLE_HEADER

#endif
Loading
Loading