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

Refactor Launch Files (and fix #50) #51

Merged

Conversation

Projects
None yet
2 participants
@rohbotics
Copy link
Member

commented Feb 25, 2018

Provides for some more future flexibility by launching

I moved teleop from a 'demo' to the base launch, since it is really part of the base services.
The next image will launch magni_bringup base.launch on boot.

Also fixes #50
Needs testing

@rohbotics rohbotics self-assigned this Feb 25, 2018

@rohbotics rohbotics requested a review from jim-v Feb 25, 2018

@ghost ghost added the in progress label Feb 25, 2018

@jim-v

jim-v approved these changes Feb 25, 2018

Copy link
Member

left a comment

This seems reasonable.

One cause of concern is that the user may launch something that conflicts with a running node. Nodes are supposed to shut down when a new node of the same name is started, but in practice there is a race condition where the old nodes releases resources in time for the new node to acquire. The only node affected in this new scheme is raspicam_node. One solution is to have the launch file run a Python script that checks if the node is running and reconfigures with appropriate parameters if it is, or starts it if not.

Another suggestion is that we remove the deprecated files, instead of adding a comment to that effect. This should remove confusion when a user stops one launch to run another, which is actually the same as the first.

@rohbotics

This comment has been minimized.

Copy link
Member Author

commented Feb 25, 2018

Another solution for raspicam_node would be to detect when the camera resource is in use and wait to grab it, but this may not be possible.

I will remove the deprecated files in a future release. The issue is that those 2 launches may be used by the systemd services on the image. Once we have a better way of distributing the service files (and updating them), we should remove teleop and joystick demos.

@rohbotics rohbotics merged commit 16a37b9 into UbiquityRobotics:indigo-devel Feb 25, 2018

@ghost ghost removed the in progress label Feb 25, 2018

@rohbotics

This comment has been minimized.

Copy link
Member Author

commented Feb 25, 2018

Merging was probably a bad idea without testing on hardware...

@jim-v

This comment has been minimized.

Copy link
Member

commented Feb 25, 2018

As a quick hack, raspicam_node could just sleep for one or two seconds on startup.

@jim-v

This comment has been minimized.

Copy link
Member

commented Feb 25, 2018

@rohbotics do you want to add the dnn_rotate demo to magni_demos?

@rohbotics

This comment has been minimized.

Copy link
Member Author

commented Feb 26, 2018

Could you add that and open a PR? Thanks

@jim-v

This comment has been minimized.

Copy link
Member

commented Feb 27, 2018

@rohbotics I ran it on a magni, and it seemed to work.

sudo service magni-base stop
roslaunch magni_bringup base.launch

Also

roslaunch magni_demos simple_navigation.launch

Running via the magni-base service also seemed to work.

Let me know if there is anything specific I should test.

@jim-v jim-v referenced this pull request Mar 6, 2018

Open

speech_command. launch #8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.