Skip to content

Commit cb33b3a

Browse files
facontidavideclaude
andcommitted
refactor(StatusChangeLogger): simplify to deferred subscription only
Remove atomic flag approach to avoid ABI break. The deferred subscription pattern (StatusChangeLogger() + subscribeToTreeChanges()) achieves the same thread safety without adding new members to the base class. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent cadda8f commit cb33b3a

3 files changed

Lines changed: 5 additions & 27 deletions

File tree

include/behaviortree_cpp/loggers/abstract_logger.h

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,7 @@ enum class TimestampType
1515
class StatusChangeLogger
1616
{
1717
public:
18-
/**
19-
* @brief Construct and immediately subscribe to status changes.
20-
* Use this for simple loggers that don't need deferred subscription.
21-
*/
18+
/// Construct and immediately subscribe to status changes.
2219
StatusChangeLogger(TreeNode* root_node);
2320

2421
virtual ~StatusChangeLogger() = default;
@@ -61,22 +58,10 @@ class StatusChangeLogger
6158
}
6259

6360
protected:
64-
/**
65-
* @brief Default constructor for deferred subscription mode.
66-
*
67-
* Use this when the derived class needs to complete initialization
68-
* (e.g., start worker threads) before receiving callbacks.
69-
* Call subscribeToTreeChanges() when ready to receive callbacks.
70-
*/
61+
/// Default constructor for deferred subscription. Call subscribeToTreeChanges() when ready.
7162
StatusChangeLogger() = default;
7263

73-
/**
74-
* @brief Subscribe to status changes on the tree.
75-
*
76-
* For loggers with threading or complex initialization, call this
77-
* at the END of your constructor after all setup is complete.
78-
* This ensures callbacks won't fire until you're ready to handle them.
79-
*/
64+
/// Subscribe to status changes. Call at end of constructor for deferred subscription.
8065
void subscribeToTreeChanges(TreeNode* root_node);
8166

8267
private:

src/loggers/bt_file_logger_v2.cpp

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ struct FileLogger2::Pimpl
4040
};
4141

4242
FileLogger2::FileLogger2(const BT::Tree& tree, std::filesystem::path const& filepath)
43-
: StatusChangeLogger() // Deferred subscription - don't subscribe yet
43+
: StatusChangeLogger() // Deferred subscription
4444
, _p(std::make_unique<Pimpl>())
4545
{
4646
if(filepath.filename().extension() != ".btlog")
@@ -81,11 +81,7 @@ FileLogger2::FileLogger2(const BT::Tree& tree, std::filesystem::path const& file
8181
flatbuffers::WriteScalar(write_buffer.data(), timestamp_usec);
8282
_p->file_stream.write(write_buffer.data(), 8);
8383

84-
// Start the writer thread
8584
_p->writer_thread = std::thread(&FileLogger2::writerLoop, this);
86-
87-
// NOW subscribe to tree changes - after everything is initialized and
88-
// the writer thread is running. This ensures callbacks are safe to handle.
8985
subscribeToTreeChanges(tree.rootNode());
9086
}
9187

src/loggers/bt_sqlite_logger.cpp

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ void execStatement(sqlite3_stmt* stmt)
5757

5858
SqliteLogger::SqliteLogger(const Tree& tree, std::filesystem::path const& filepath,
5959
bool append)
60-
: StatusChangeLogger() // Deferred subscription - don't subscribe yet
60+
: StatusChangeLogger() // Deferred subscription
6161
{
6262
const auto extension = filepath.filename().extension();
6363
if(extension != ".db3" && extension != ".btdb")
@@ -129,9 +129,6 @@ SqliteLogger::SqliteLogger(const Tree& tree, std::filesystem::path const& filepa
129129
}
130130

131131
writer_thread_ = std::thread(&SqliteLogger::writerLoop, this);
132-
133-
// NOW subscribe to tree changes - after everything is initialized and
134-
// the writer thread is running. This ensures callbacks are safe to handle.
135132
subscribeToTreeChanges(tree.rootNode());
136133
}
137134

0 commit comments

Comments
 (0)