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

Changes include the addition of tolerance to network interruptions and the option of speed or position control for PTZ functions #21

Merged
merged 11 commits into from
May 15, 2013

Conversation

paulmilliken
Copy link

I have forked the axis_camera package on github and made a few changes:

  • I added code to address issue Handle loss of access to the camera. #1 by polling the camera every few seconds until it reappears.
  • I addressed issue separate executable scripts factoring out common code into modules #15 by removing the axis_twist.py script and modifying axis_ptz.py to allow the user to choose between position control and speed control. Speed control is likely to be more appropriate for teleoperation. A Twist message could still be used to control speed, by writing a simple ros node to subscribe to Twist and translate to an Axis message (using speed control in the axis_ptz.py node). This "translator" node would also provide a place to apply controller gains for some kind of slow autonomous control loop.
  • I addressed Issue axis.py: log formatted exception information rather than printing it #19 by logging errors with rospy.logwarn and rospy.loginfo
  • I changed the tab size from 2 spaces to 4 spaces. It's just my personal preference for python coding so, if you choose to include my changes, feel free to change it back to whatever you prefer.
  • I added some launch files to test the code. The most interesting is test_speed_control.launch (which also requires https://github.com/paulmilliken/teleop_view.git which is based on code for the image_view node)
  • There were quite a few changes so I incremented the version number in my fork.
  • I haven't touched or tested the publish_axis_tf.py node at all.
    I'm interested to hear you thoughts, especially if you think something could have been implemented in a better way.
    Paul

jack-oquin and others added 10 commits April 9, 2013 22:03
Just README and CHANGELOG, no Python documentation yet.
…z.py now has option for speed_control or position_control. teleop_twist.py and axis_twist.py have been removed as they are superceded by speed control option in axis_ptz.py. Mirror function added to axis_ptz.py in addition to flipped. A few more test launch files added but more are required.
@jack-oquin
Copy link
Member

A fix for #1 is extremely welcome. Thanks, Paul!

Note that there is considerable overlap between this and pull request 17. We should probably do some work to merge those two versions.

@paulmilliken
Copy link
Author

Hi Jack,
Regarding issue #1 , you're welcome.
I had a look at pull request #17 and it looks like a merge will be relatively straightforward once I gain an understanding of Sphinx so I'll aim to look at it next week.

@jack-oquin
Copy link
Member

There's not much Sphinx logic there, just the basic rosdoc configuration and an index.rst that includes the README and CHANGELOG.

My intention is to add in-line Sphinx documentation to the Python modules, but none of that is done yet.

@paulmilliken
Copy link
Author

Jack, I just merged your pull request #17 and committed to https://github.com/paulmilliken/axis_camera.git. There were very few conflicts to resolve. However, I have assumed that axis_twist.py, teleop_twist.py and axis_all.py will be replaced by the extended functionality of axis_ptz.py which now has the option of either speed control or position control. I thought this was the tidiest way to avoid replication of similar functions in different files but I welcome feedback if this approach is not to everyone's taste.

@jack-oquin
Copy link
Member

Sounds fine. I'm glad it wasn't difficult.

Maybe I should cancel #17, to avoid confusion?

@paulmilliken
Copy link
Author

Ok. That sounds sensible as long as you are happy with the way I merged the changes.

@jack-oquin
Copy link
Member

It's fine. I made a copy/paste error in the title of the index.rst, but it's not worth fixing right now.

Pull requests that sit in the queue for a month are a drag on productivity.

mikepurvis added a commit that referenced this pull request May 15, 2013
Changes include the addition of tolerance to network interruptions and the option of speed or position control for PTZ functions
@mikepurvis mikepurvis merged commit 3203fbe into ros-drivers:master May 15, 2013
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