Add in a warning for a KeepLast depth of 0.#2048
Conversation
There was a problem hiding this comment.
I usually think that user-specified settings that do not make sense should result in immediate crashes that are not possible to ignore, to make the robot fail as soon as possible.
However, here there's an additional complexity: what happens if someone changes the QoS at runtime using the qos_parameters_overrides?
In this case we should rather reject the change as we don't want people to be able to kill robots that are already running.
This probably is out of scope for the tiny change that you did here.
The parameters overrides are only read once at startup, they cannot be changed aferwards. |
It really doesn't make much sense to have a KeepLast depth of 0; no data could possibly be stored. Indeed, the underlying DDS implementations don't actually support this. It currently "works" because a KeepLast depth of 0 is assumed to be system default, so the RMW layer typically chooses "1" instead. But this isn't something we should be encouraging users to do, so add a warning. Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
d333afd to
159892a
Compare
|
CI:
|
|
CI is clean here, so merging. |
It really doesn't make much sense to have a KeepLast depth of 0; no data could possibly be stored. Indeed, the underlying DDS implementations don't actually support this. It currently "works" because a KeepLast depth of 0 is assumed to be system default, so the RMW layer typically chooses "1" instead. But this isn't something we should be encouraging users to do, so add a warning.
Signed-off-by: Chris Lalancette clalancette@openrobotics.org
I'll note that I could easily be convinced to make this throw an exception instead. That would be a much stronger warning to users not to do this, but it does break existing practice so has some risk of downstream breakage. Happy to discuss it.