lucasw / rviz_camera_stream

Custom rviz camera plugin that published rendered camera video stream
BSD 3-Clause "New" or "Revised" License
59 stars 47 forks source link

Added option to throttle frame rate and trigger single images #9

Closed jwendelmuth closed 8 years ago

jwendelmuth commented 8 years ago

Added ability to throttle framerate using a parameter /rviz_camera_framerate (Absolute name as RVIZ is started anonymously and other nodes need a fixed name). If framerate is set to 0, no images are published.

It is also possible to trigger a single image by sending a std_srvs/Trigger on /rviz_camera_trigger.

lucasw commented 8 years ago

It looks like this can only have framerates that can be multiplied by an integer to equal ~30 (the base rviz framerate)- like 1,2,3, 5,6, 10, and 15 but nothing in-between.

That might be desirable in cases where a fixed interval between frames is desired, another approach could be to update 'last_publicationtime' to last_publicationtime + 1.0 / frame_rate instead of the current time- the output frame rate will be uneven but average out to closer to what the user wants. (The initial value of last_publicationtime would need to be set to the initialization time rather than zero also) (Also #7 would go well with that change.) Maybe there could be a checkbox to switch behaviors.

That isn't necessary to add for this merge request, I'm just recording the idea here. I'll probably make a few edits and then merge this either today or Monday.

lucasw commented 8 years ago

I made some additional formatting changes and squashed into one commit 7839c3a , thanks!

As far as rviz being launch anonymous, it is a better practice to launch it with a name and then multiple rviz instance can exist without having to share the same trigger service call, so I'll likely change that to a non-absolte service later. (does it even work to have two nodes with the same service? Do they both get called?).

lucasw commented 8 years ago

Not sure if there is a more proper way to edit and merge and close the way I did and make the pull request look like it was merged.

Also if you make additional changes you'll have to clobber your current master with my master and lose your old history, it's better to have one named feature branch per pull request and have your master track upstream.

NikolasE commented 8 years ago

" Do they both get called?)."

That's what I also thought about this weekend. I think we should add a name so that we can use several camera streams (could be very useful to create a video with a bird's eye view and one view from the robot)

lucasw commented 8 years ago

It certainly doesn't work to have the same node with two instances trying to advertise the same service:

[ERROR] [/rviz] [/tmp/binarydeb/ros-kinetic-roscpp-1.12.2/src/libros/service_manager.cpp]:[147] [Tried to advertise a service that is already advertised in this node [/rviz_camera_trigger]]
NikolasE commented 8 years ago

I created a new PR to deal with this issue