Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added master-slave configuration #135

Closed

Conversation

dariodenardi
Copy link

I fixed a small bug in stereo_synced.launch.py and added a master-slave configuration

Copy link
Collaborator

@berndpfrommer berndpfrommer left a comment

Choose a reason for hiding this comment

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

camera_params2 has a lot of code replication wrt camera_params1. Could you please clean that up?

@berndpfrommer
Copy link
Collaborator

Also please fix the formatting errors in the launch script.

@berndpfrommer
Copy link
Collaborator

One of the checks failed due to a copyright formatting issue that was not caused by your commits, i.e. that was pre-existing and somehow not flagged before. My most recent commits fix that, along with other python formatting issues that unfortunately are currently not flagged by the CI tests. So please rebase to master before fixing above issues.

Copy link
Collaborator

@berndpfrommer berndpfrommer left a comment

Choose a reason for hiding this comment

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

With the new code, are all the parameters set for camera2? For example "gain" does not seem to be set.
I suggest the following: rather than having the conditional for "master" in make_camera_node(), pass in the camera parameters as an an argument (instead of arch_type). Pass in different camera parameters for camera2 than for camera1.
As for the camera parameters: use the camera parameters for e.g. camera1, and replace the values that differ for camera2. That makes it clearer what the difference is between master and slave. You might find this link helpful for how to replace dict values:
https://stackoverflow.com/questions/19773669/python-dictionary-replace-values

@berndpfrommer
Copy link
Collaborator

With the new synchronized camera driver in place, this PR has become obsolete.

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.

2 participants