-
Notifications
You must be signed in to change notification settings - Fork 16
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
In Vizkit::ConnectionManager: Using port as key leads to duplicates #68
Comments
This would definitely not make sense. Objects that are different should look different from the API perspective. Changing the notion of equality globally for a specific use-case would be a really bad choice. Disclaimer: this is not my code, so the following comes from what I remember about discussions with @D-Alex. The sub-port proxying is there so that fields from port and full ports looks all the same from the listening side - which allows Vizkit to display e.g. an image that is a field within a bigger structure. Nonetheless, a given port proxy will only create a single connection and be polled only once, regardless of the number of listeners (full or partial) that are on it, i.e. it's not more costly to have it the way it currently is. orocos.rb is doing the "reference counting", so that Vizkit can stop the listeners when it needs to, and have the connection severed only when all listeners stopped listening. Some evidence supporting this: code-wise, SubPortProxy is actually a delegate on top of the "full" port. SubPortProxy#on_raw_data calls |
The hash @on_data_listeners in Vizkit::ConnectionManager uses port as key. However, if the port is e.g. of type SubPortProxy, and if the user connects again to the same port (determined from type, task, and name), the port is added multiple times (and we get multiple listeners for the same data). Is this the intended behavior? When should we consider
port
to be equal?I found that
Orocos::PortBase
has a definitionWould adding a similar function as e.g.
SubPortProxy.eql?
(hash uses key.eql?) be reasonable?The text was updated successfully, but these errors were encountered: