Skip to content

Foundation: Add AsyncNotificationCenter#4957

Merged
matejk merged 18 commits intomainfrom
4851_async-notification-center
Jun 12, 2025
Merged

Foundation: Add AsyncNotificationCenter#4957
matejk merged 18 commits intomainfrom
4851_async-notification-center

Conversation

@matejk
Copy link
Copy Markdown
Contributor

@matejk matejk commented May 27, 2025 •
edited
Loading

Resolves #4851.

  • workers that dispatch notifications to observers
  • ability for synchronous dispatch of a notification
  • more unit tests
  • Obsolete Poco::Observer in favour of Poco::NObserver
    • Replace in Poco::Net
    • Replace in Foundation tests
  • improve notification matching (avoid dynamic_cast is not necessary, add std::function for matcher)

@matejk matejk requested a review from aleks-f May 27, 2025 07:43
@matejk matejk force-pushed the 4851_async-notification-center branch 2 times, most recently from 6e5b34d to b95bd92 Compare May 27, 2025 10:25
aleks-f
aleks-f requested changes May 27, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@aleks-f aleks-f left a comment

Choose a reason for hiding this comment

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

mostly style review

github-advanced-security[bot]
github-advanced-security bot found potential problems May 27, 2025
View reviewed changes
@matejk matejk force-pushed the 4851_async-notification-center branch 4 times, most recently from 84b0d23 to 6ff22ae Compare May 28, 2025 12:22
@matejk matejk requested a review from pavledragisic May 28, 2025 13:26
@matejk
Copy link
Copy Markdown
Contributor Author

matejk commented May 28, 2025

Implemented synchronous dispatch of notifications and collecting results.

@aleks-f , @pavledragisic , please take a look and comment.

@matejk matejk requested a review from aleks-f May 28, 2025 13:27
@matejk matejk force-pushed the 4851_async-notification-center branch from c3fb6f6 to 5e585e9 Compare May 28, 2025 13:42
@matejk matejk mentioned this pull request May 29, 2025
Closed
@matejk matejk marked this pull request as ready for review May 29, 2025 07:41
matejk added 15 commits June 10, 2025 15:24
@matejk
feature(AsyncNotificationCenter): introduce worker pool that dispatch…
fbe01e3 
…es notifications to observers asynchronously.
@matejk
chore(c++20): Conditional sections of code to enable features in Asyn…
d2f577b 
…cNotificationCenter.
@matejk
chore(AsyncNotificationCenter): organised includes.
84c80d2
@matejk
enh(AsyncNotificationCenter): unit tests, more documentation and impr…
f955da1 
…ovements.
@matejk
chore(AsyncNotificationCenter): updates from code review.
05df0c4
@matejk
chore(github ci): Use newer OS (macOS, Ubuntu) for some of the actions.
cd5ed48
@matejk
enh(AsyncNotificationCenter): Only start threads that are required.
d91c8b5
@matejk
chore(Poco::Observer): declare Poco::Observer deprecated in favour of…
b6ffd06 
… NObserver.
@matejk
enh(NObserver): use std::function as matcher in addition to plain poi…
c0c3b8a 
…nter to an object's function, avoid dynamic_cast when observer has a matcher.
@matejk
feature(AsyncNotificationCenter): introduce synchronousDispatch(..) t…
8ecd94d 
…o dispatch a notification synchronously to observers and return results.
@matejk
fix(c++17): Fix compile error for AsyncNotificationCenter.
1cdcdb2
@matejk
fix(NObserver): matcher callback handler was not called
a61037c
@matejk
enh(Net): Replace Poco::Observer with Poco::NObserver.
0ff26a1
@matejk
chore(Foundation): Replace Poco::Observer with Poco::NObserver in Tas…
3969503 
…kManagerTest.
@matejk
enh(AsyncNotificationCenter): update class documentation.
ef8539d
@matejk matejk force-pushed the 4851_async-notification-center branch from b9ba74e to 85527a9 Compare June 10, 2025 13:29
aleks-f
aleks-f approved these changes Jun 12, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@aleks-f aleks-f left a comment

Choose a reason for hiding this comment

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

just shorten all occurrences of Synchronous* names to Sync. the rest looks ok

@matejk matejk merged commit 735e174 into main Jun 12, 2025
80 checks passed
@matejk matejk deleted the 4851_async-notification-center branch June 12, 2025 19:33
@matejk matejk changed the title Async notification center (dispatch notifications asynchronously) Foundation: Add AsyncNotificationCenter Dec 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aleks-f aleks-f aleks-f approved these changes

@pavledragisic pavledragisic Awaiting requested review from pavledragisic

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Foundation: AsyncNotificationCenter should dispatch asynchronously

2 participants

@matejk @aleks-f