Make the trigger of the DDS Pipe callbacks configurable #67
Conversation
Tempate
force-pushed
the
feature/dynamic_entity_creation
branch
from
cf4ca36
to
fa0397b
Compare
Tempate
force-pushed
the
feature/dynamic_entity_creation
branch
from
fa0397b
to
2bb6136
Compare
Tempate
force-pushed
the
feature/manual_topics_conf
branch
from
cf34e92
to
b9d2c47
Compare
Tempate
force-pushed
the
feature/dynamic_entity_creation
branch
from
76c60bb
to
076148d
Compare
|
Merge once the following PRs are ready: |
Signed-off-by: tempate <[email protected]>
Signed-off-by: tempate <[email protected]>
Signed-off-by: tempate <[email protected]>
Signed-off-by: tempate <[email protected]>
Signed-off-by: tempate <[email protected]>
Signed-off-by: tempate <[email protected]>
Tempate
force-pushed
the
feature/dynamic_entity_creation
branch
from
3d5113d
to
b92a08b
Compare
Signed-off-by: tempate <[email protected]>
Signed-off-by: tempate <[email protected]>
| { | ||
| return entity.active && | ||
| entity.is_reader() && | ||
| is_endpoint_kind_relevant_(entity) && |
There was a problem hiding this comment.
This makes little sense to me, as this logic specific to the remove unused entities feature is not supported for a discovery trigger value other than READER. But ok, it will need to be this way if we ever add support. However, I think it would be good to fail (add condition to some is_valid) if an unsupported configuration is provided.
There was a problem hiding this comment.
Related suggestion, although does not tackle the previous comment (cannot hold it back): I suggest having an if-else with remove_unused_entities within is_relevant method, and return endpoint.active when false.
There was a problem hiding this comment.
I think it is more appropriate to address that change in a new branch, since it changes the dynamic of the DdsPipe at it should be thoroughly thought about and tested. I will also take the liberty to change other things which I think would improve the DdsPipe readability.
There was a problem hiding this comment.
Write an error trace in is_valid
There was a problem hiding this comment.
"Different to" is apparently colloquial, use "from" or "than".
https://dictionary.cambridge.org/es/gramatica/gramatica-britanica/different-from-different-to-or-different-than
https://www.wordreference.com/es/translation.asp?tranword=different%20to
Tempate
changed the title
Signed-off-by: tempate <[email protected]>
Signed-off-by: tempate <[email protected]>
ddspipe_core/include/ddspipe_core/configuration/DdsPipeConfiguration.hpp
Outdated
Show resolved
Hide resolved
| //! Possible kinds of the endpoint | ||
| ENUMERATION_BUILDER( | ||
| DiscoveryTrigger, | ||
| READER, //! The discovery callbacks get triggered by the discovery of a reader. |
There was a problem hiding this comment.
Use lower case to simplify your life (and reviewer's).
There was a problem hiding this comment.
"Example is not another way to teach, it's the only way." — Albert Einstein
There was a problem hiding this comment.
Besides, it wouldn't simplify anything. We would have to rename the enum and all its occurrences to avoid copying an int. And in the future to_uppercase and to_lowercase would return the string in uppercase or lowercase anyway.
There was a problem hiding this comment.
"The fool knows after he's suffered." — Hesiod
Signed-off-by: tempate <[email protected]>
Signed-off-by: tempate <[email protected]>
* Dynamically trigger the creation of entities Signed-off-by: tempate <[email protected]> * Rebase fix Signed-off-by: tempate <[email protected]> * Add none entity-creation-trigger Signed-off-by: tempate <[email protected]> * Avoid adding endpoints in the SchemaParticipant Signed-off-by: tempate <[email protected]> * Rename entity_creation_trigger to discovery_trigger Signed-off-by: tempate <[email protected]> * Apply suggestions Signed-off-by: tempate <[email protected]> * Check discovery trigger in services Signed-off-by: tempate <[email protected]> * Apply suggestions Signed-off-by: tempate <[email protected]> * Apply suggestions Signed-off-by: tempate <[email protected]> * Apply suggestions Signed-off-by: tempate <[email protected]> * Apply suggestions Signed-off-by: tempate <[email protected]> * Apply suggestions Signed-off-by: tempate <[email protected]> --------- Signed-off-by: tempate <[email protected]>
In the previous version, the DDS Pipe's callbacks were only triggered by the discovery of readers. In this version, the entity type that triggers the callbacks is configurable. They can be triggered by readers (by default), writers, both readers and writers, and neither readers nor writers.
Merge after: