SteveMacenski / spatio_temporal_voxel_layer

A new voxel layer leveraging modern 3D graphics tools to modernize navigation environmental representations
http://wiki.ros.org/spatio_temporal_voxel_layer
GNU Lesser General Public License v2.1
616 stars 183 forks source link

Fix "enable" parameter for observation sources #252

Closed HovorunB closed 1 year ago

HovorunB commented 1 year ago

I noticed that if we set enabled: false for observation sources it doesn't actually disable them on startup. I think the problem is that enabled parameter is responsible for creating / destroying subscription only on service call, and is not considered for the subscription creation on startup. Or am I missing smth?

In this PR I propose to add checking for enabled parameter on startup to make sure that the observation source is not subsrcibed to the topic if it is disabled

Another approach could be to use this implementation In short: Create subscription objects without actually subscribing to the topic, then subscribe to the topics from activate function and only for observation sources which are enabled

Wanted to get your thoughts about implementation. Maybe you have other ideas?

HovorunB commented 1 year ago

Sorry for the delay, at first i tried to solve this in a way how (under my assumption) it was done before (when creating a subscription object didn't automatically subscribe to the topic). But now i see that this might not be a very good solution. Then I suggest another one, to check if observation source in enabled on callback, and return otherwise. This way we don't rely on length of sub_it and buf_its, and keep the logic on reactivation