Skip to content

Camera capture#27

Draft
Raytesnel wants to merge 18 commits intomainfrom
camera_capture
Draft

Camera capture#27
Raytesnel wants to merge 18 commits intomainfrom
camera_capture

Conversation

@Raytesnel
Copy link

@Raytesnel Raytesnel commented May 11, 2023

Snap shot, live stream within one node. both uses the same gstreamer pipline and can be runned at the same time.
enable snapshot or livestream on trigger/state topics.

  • Make a HSL stream node
  • Change Readme
  • Remove lagacy folders (eg host.rtsp.server.c & ros2_csi_camera_publish)
  • Update launch file from robot_app --> updated with skeleton @WPDOrdina and remove lagacy nodes.

@Raytesnel Raytesnel requested a review from WPDOrdina May 11, 2023 07:38
@Raytesnel Raytesnel self-assigned this May 11, 2023
.gitignore Outdated
/venv/
/.idea/
log/
install/ No newline at end of file

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you move these to the top-most "section"? That keeps the .gitignore (kind of) organised

@Raytesnel
Copy link
Author

finished enough for the event (@RemcoMusic ), only we need to check if it works to have a livestream on the front end before merging. We can make it nicer after the event.

Copy link

@WPDOrdina WPDOrdina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A quick looks tells me that the code is fine for now. As discussed, we will look into this more after the two upcoming events.

Please take a look at my comments and make some small fixes. After that, let's merge!

with open("livestream/output.mp4", mode="rb") as stream_file:
yield from stream_file
return StreamingResponse(iter_file(), media_type='text/event-stream')
# return StreamingResponse(iter_file(), media_type='video/mp4') No newline at end of file

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commented code can be removed

cd ~/robotics-workshop
chmod +x host_rtsp_server
colcon build --symlink-install && source install/local_setup.bash
colcon build && source install/local_setup.bash

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I propose to keep the "--symlink-install" in the build-command, as it should keep the amount of times to build the code again to a minimum

self.image_location = f"{json_settings['image_location']}"
self.image_counter = 0

def start_livestream_callback(self, topic_msg: Bool):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function name does not really reflect its function, as it starts and stops the livestream. Would livestream_state_callback be an option for you?

self.get_logger().debug(
"can't stop livestream because livestream is already stopped"
)
elif not self.cap.isOpened():

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An 'else`-statement would suffice

timer.destroy()
except UnboundLocalError:
self.get_logger().error("timer was is not defined)")
else:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When would this else-statement be reached? It seems topic_msg.data is always True or False, so this is never reached right?

)

return LaunchDescription([
# launch_include,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commented code

self.publisher_ = self.create_publisher(Twist, publish_topic, 1)
self.publisher2 = self.create_publisher(String, '/trigger', 1)
self.pub_joystick = self.create_publisher(Twist, publish_topic, 1)
self.pub_camera_trigger = self.create_publisher(Bool, '/camera/cam_0/snapshot/trigger', 2)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally these are changed to variable arguments from the configuration file, but lets not do that now

configuration = json.load(config_file)


# Define launch arguments that can be set from the command line using

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These comment lines can be removed

Image, topic, self.listener_callback, queue_size
)
self.bridge = CvBridge()
# os.environ['DISPLAY']=':0'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

COmment

)
self.out.write(frame)
self.out.release()
# cv2.imshow("frame", frame)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comments

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants