ros-drivers / axis_camera

Contains basic Python drivers for accessing an Axis camera's MJPG stream. Also provides control for PTZ cameras.
BSD 3-Clause "New" or "Revised" License
52 stars 74 forks source link

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

Closed paulmilliken closed 11 years ago

paulmilliken commented 11 years ago

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

jack-oquin commented 11 years ago

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 commented 11 years ago

Hi Jack, Regarding issue https://github.com/clearpathrobotics/axis_camera/issues/1 , you're welcome. I had a look at pull request https://github.com/clearpathrobotics/axis_camera/pull/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 commented 11 years ago

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 commented 11 years ago

Jack, I just merged your pull request https://github.com/clearpathrobotics/axis_camera/issues/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 commented 11 years ago

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

Maybe I should cancel #17, to avoid confusion?

paulmilliken commented 11 years ago

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

jack-oquin commented 11 years ago

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.