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

Support for multiple CBs subscribed to single client #73

Closed
yassiezar opened this issue Nov 21, 2022 · 2 comments
Closed

Support for multiple CBs subscribed to single client #73

yassiezar opened this issue Nov 21, 2022 · 2 comments

Comments

@yassiezar
Copy link
Contributor

yassiezar commented Nov 21, 2022

We have an interesting problem. We have a client handling all incoming and outgoing HTTP traffic - CBs request it to make async requests and then feeds the response back to the CB via the SMACC signal system. However, I've run into a problem where both CBs' callbacks are triggered on each HTTP response, regardless of whether the original request came from a particular CB or not.

My research seems to show that this is a limitation of the slots and signals system underpinning SMACC's own signal system. However, I found a hacky workaround for our use-case where you grab the connection returned from the onResponseReceived() override, create a boost::shared_connection_block from it and unblock/block it before and after making requests to the client. We currently have low latency networks, but this workaround will likely break on slower ones.

My question: is there a nicer way to implement this behaviour? I.e. only trigger callbacks on certain subscribed CBs.

┆Issue is synchronized with this Jira Task by Unito

@pabloinigoblasco
Copy link
Collaborator

Hello yassiezar. Thanks for your comment.
I am thinking about your issue. I am asuming the code you refer is this based on open PR (https://github.com/robosoft-ai/SMACC/pull/55/files).

I think the discussion around is about concepts rather than implementations or limits:

The concept of a "SmaccSignal" is associated to some internal change on the "owner". All the subscribers objects are notified of this change (this is essentially the observer pattern. SMACC implements that concept. Therefore, if you only have one single client or "owner" of the signal, all the subscribers will receive the same notifications at the same time.

Maybe you are looking for some more advanced concept. Something like a "signal dispatcher". If you need to dispatch different signals of the owner to different subscribers you may need to use a vector of signals or a map of signals in the owner.

Concerning your workaround, I am not sure how it works, maybe with some code reference I could have a look to understand better.

What do you think about this point of view?

@yassiezar
Copy link
Contributor Author

yassiezar commented Nov 28, 2022

Hey @pabloinigoblasco, that's right, its indirectly related to #55

That's fair enough. I think I just wanted to make sure that its indeed the case that all observers are notified or if there was a way to somehow direct the signal to specific units or entities (the Boost docs weren't very specific). A signal dispatcher would probably be the best solution here, but its not immediately clear to me how this could be implemented. The signals don't carry information on the originating CB object, for example, so its not clear how I might trigger the correct callback based on the signal alone.

For your reference, the code I used is similar to the following and is similar to using mutex locks attached to the signal connection (imagine that block_, connection_ and client_ were class members defined elsewhere).

void Cb::onResponseReceived() {
  block_.unlock();
  foo();
  block_.lock();
}

void Cb::onEntry() {
  this->requiresClient(client_);
  connection_ = client->onResponseReceived(&Cb::onResponseReceived, this);
  block_ =  boost::signals2::shared_connection_block{connection_};
}

Anyway, I'll think about it a bit more and maybe extend the next version of #55 with a fix if its appropriate. For now I'll close this as its not really related to SMACC itself

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants