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 72 forks source link

Features added & major rewrite. #42

Open peci1 opened 8 years ago

peci1 commented 8 years ago

Hi guys, in our TRADR project we use an Axis 214 PTZ cam on a mobile robotic platform running ROS.

So we started using this driver, but as time went, we needed more features than it currently provides.

We were thinking about the best way how to add the features we need, and finally we agreed that the current API has some flaws (e.g. mixing focus/brightness control together with pan/tilt/zoom) we would also like to fix.

So we started a complete rework of the driver, and we achieved the following as a result:

Please, take a look at the new code (looking at the changelog is probably not a good idea since we changed too much). Tell us if you like the changes we made, and, please, test it with your existing applications utilizing the old API to be sure the backwards-compatibility layer works as expected.

Also, please, tell us if the backwards compatibility layer is needed at all. If it weren't, some more structural changes towards clean design could be done.

Thank you very much and hope you'll like it.

awesomebytes commented 8 years ago

Hello, I'm writing just to let you know I've seen your PR and I find it great all you've done. I'm currently traveling and it's complicated for me to give it a proper look. I also wanted to give it some love (and internally we are using some modified stuff at PAL) but had no time to make it pretty and not breaking compatibility.

I expect I'll have some spare time next week. I would like to try the package with a real camera before merging. That would take another week, but if someone else can verify its correctness I'll be glad to merge the changes. On Oct 11, 2015 9:12 PM, "Martin Pecka" notifications@github.com wrote:

Hi guys, in our TRADR project we use an Axis 214 PTZ cam on a mobile robotic platform running ROS.

So we started using this driver, but as time went, we needed more features than it currently provides.

We were thinking about the best way how to add the features we need, and finally we agreed that the current API has some flaws (e.g. mixing focus/brightness control together with pan/tilt/zoom) we would also like to fix.

So we started a complete rework of the driver, and we achieved the following as a result:

  • The driver is ready to be used with more Axis camera types than just the 214 PTZ (because we implemented a large part of the VAPIX API communication).
  • There is a camera model displayable in RViz, the camera reports its pan/tilt/zoom as both a PTZ message, JointStates message, and via TF
  • More parameters controllable via dynamic_reconfigure: video stream parameters (resolution, FPS etc.)
  • Code structure changes for "better" (in our view, of course) support of current Python and OOP recommendations.
  • Renamed methods and members according to PEP 8 Python naming standard (if it's there, why not use it)
  • We tried as hard as we could to retain backwards compatibility with the previous version (by adding extra backwards compatibility layers translating old code usage into the new one). We haven't renamed any topics from the current version. The camera stream is published on the same topic in the new code, and the PTZ-related topics are still there, just deprecated.

Please, take a look at the new code (looking at the changelog is probably not a good idea since we changed too much). Tell us if you like the changes we made, and, please, test it with your existing applications utilizing the old API to be sure the backwards-compatibility layer works as expected.

Also, please, tell us if the backwards compatibility layer is needed at all. If it weren't, some more structural changes towards clean design could be done.

Thank you very much and hope you'll like it.

You can view, comment on, or merge this pull request online at:

https://github.com/ros-drivers/axis_camera/pull/42 Commit Summary

  • Fixed ROS dependencies in CMakeLists.txt and package.xml.
  • Refactoring, added compression and FPS control support.
  • Added missing std_msgs dependency to package.xml.
  • Enabled dynamic_recofigure for compression, FPS and resolution.
  • Added support for pausing/resuming the camera stream, and to wake up a standby camera.
  • Refactored camera wakeup call and checked if the wakeup URL is correct.
  • Refactored the dynamic reconfigure and message definitions. The old ones are retained for BC.
  • Moved the API communication and video streaming to separate classes.
  • Improved the camera wakeup behavior.
  • Added support for camera restart.
  • Improved backwards compatibility with the original library.
  • Renamed streaming.py to video_streaming.py
  • Refactored camera position streaming to separate module.
  • Implemented most of the VAPIX PTZ functions.
  • Added dynamic_reconfigure logic to the camera controller.
  • Added look_at support.
  • Added keyboard teleop node.
  • Fixed dynamic reconfigure issues.
  • joint_states message is now correctly published in radians.
  • Added the 3D visual and collision model of the camera. This also enabled TF publishing.
  • Added the ability to take snapshots.
  • Now the defaults are set up so that no user authentication is performed (requires anonymous PTZ control, or providing credentials).
  • Small refactoring, improved documentation.
  • Finished documenting and refactoring vapix.py.
  • Documented axis.py.
  • VAPIX.get_parameter() was using wrong method to get the API response
  • the hostname was therefore prepended twice.
  • Fixed some bugs.
  • Allowed resolutions are now reported in a parameter.
  • Documented and refactored video_streaming.py.
  • Documented and refactored camera_control.py.
  • Corrected CRLF to LF.
  • Documented nodes. Small refactorings.
  • Fixed some omitted refactorings.
  • Fixed typo in launch file.
  • Changed default state publishing frequency to 50 so that TFs are working correctly.
  • Fixed some problematic situations where urllib2 doesn't automatically wrap socket.timeout exception as URLError.
  • Fixed some more refactoring bugs. Now everything seems to be working as it should.
  • Fixed the makefile.
  • Corrected index.rst package name.
  • Added documentation for the new API.
  • Fixed typo.
  • Fixed topic datatypes for focus, iris and brightness.

File Changes

Patch Links:

— Reply to this email directly or view it on GitHub https://github.com/ros-drivers/axis_camera/pull/42.

peci1 commented 8 years ago

@awesomebytes: Thanks, I'm looking forward for hearing feedback from you or other users of this package.

awesomebytes commented 8 years ago

I'm testing the PR as I type. I really like it.

I found out that if I send a velocity command on pan or tilt and then I send a 0 the camera keeps moving. If I send a 0.000001 it stops.

I'll review the usage with older launchfiles to see if everything keeps working in my system and post again.

awesomebytes commented 8 years ago

I succesfully merged this version and tried it with a system that uses it and it works. Although I'm not using it as a source of the video stream (for that I use video_stream_opencv which gives me better frame rate and less cpu usage) but the PTZ is sweet, commanding in speed gives a way better experience (adapted an older node which I hope to add at some point to this package).

Can someone else try with an existing setup?

peci1 commented 8 years ago

Thanks very much for the tests, @awesomebytes. Could you please test the pan/tilt velocity problem by issuing a direct API command in a browser? If you don't know how to compose the API URL, change logging level to DEBUG in rqt_console for the ptz node, and on debug level it prints out the API URLs it calls.

awesomebytes commented 8 years ago

Hey I tried, If I send a 0.0 I get:

Calling PTZ command continuouszoommove=0 on host 10.68.0.6, camera 1

Opening VAPIX URL http://10.68.0.6/axis-cgi/com/ptz.cgi?camera=1&continuouszoommove=0 .

If I send 0.00001:

Calling PTZ command continuouspantiltmove=0,0&continuouszoommove=0 on host 10.68.0.6, camera 1

Opening VAPIX URL http://10.68.0.6/axis-cgi/com/ptz.cgi?camera=1&continuouspantiltmove=0,0&continuouszoommove=0 .

Seems like the 0.0 is treated as a "not filled field" value, or something.

peci1 commented 8 years ago

@awesomebytes Thanks for the info, I hope the bug is fixed now. Please, try to update your branch and test it again.

jeff-o commented 8 years ago

I've been watching these changes with interest since I use Axis PTZ cameras quite often. I tried using the driver with an Axis P5512-E, and unfortunately it did not work. If the plan is to merge these changes in with the main branch then it will either need to be differentiated as being exclusively for the Axis 214, or P5512 support will need to be built in.

In a week or so I'll be receiving a new Axis 214 camera to try out with this driver, I'll let you know how it goes.

On Thu, Oct 29, 2015 at 9:00 PM, Martin Pecka notifications@github.com wrote:

@awesomebytes https://github.com/awesomebytes Thanks for the info, I hope the bug is fixed now. Please, try to update your branch and test it again.

— Reply to this email directly or view it on GitHub https://github.com/ros-drivers/axis_camera/pull/42#issuecomment-152370708 .

Jeff Schmidt System Integration Technologist Clearpath Robotics Phone: 519-513-2416

peci1 commented 8 years ago

Jeff, according to the documentation, your P5512-E camera should also support the VAPIX API and therefore should be supported by the driver.

I think we can debug the cause. I just don't think we need to do the communication here. I left you an email message and let's continue there ;)

peci1 commented 8 years ago

I did an architectural change to the way how resolutions are handled. Because there are going to be numerous resolutions for different Axis products, it seemed to me impractical to create a list of supported resolutions, from which only a few would be supported by particular camera models.

So in the last 2 commits I did a little hack that exploits the dynamic_reconfigure API and dynamically generates the list of supported resolutions based on what the camera reports. It is non-standard code, but I tested it causes no problems when using either dynparam, rqt_reconfigure or the Python client.

I also improved the error messages when connecting to the API doesn't succeed. Now, it the camera returns 401 Unauthorized, the log message instructs the developer to provide a username/password or to allow anonymous access.

cclauss commented 5 years ago

Status?

peci1 commented 5 years ago

Conflicts of this branch with master should be easy to resolve. However, I'm still not sure the maintainers (if there are still any) are interested in this different approach. Maybe forking and creating an alternative driver would be better?

civerachb-cpr commented 4 years ago

Hi, this package was orphaned for a long time. Clearpath has just (re-)assumed maintainership. We're looking into this now and will likely be merging it soon.

peci1 commented 4 years ago

Wow. Haven't expected anyone will ever resurrect this :) I'm no longer actively using the camera, so I can't tell whether the code still works or if it is compatible with newer models.

civerachb-cpr commented 4 years ago

We'll make sure things are still working before merging things into master. But given how long this project has been dormant, any improvements are welcome. For now I've merged the changes into a new branch (pr-42) so we can test things out more. Assuming that goes well we should be able to finally close out this PR.

civerachb-cpr commented 4 years ago

I finally had a chance to try out your changes with an Axis M5525 Unfortunately it appears there are some bugs that are preventing the driver from properly detecting the camera settings: launching the node gives the following errors & warnings:

`[WARN] [1590506707.956746]: Could not determine resolutions supported by the camera. Asssuming only CIF.

[...]`

The HTTPError messages only start appearing once I add the image topic in rviz. The current master & 0.3.0 versions do work correctly with this camera.

peci1 commented 4 years ago

Thanks for testing. Could you try to get some deeper debug info? What are the HTTP errors about? Can you try to change the source code so that the content of the error is displayed?

civerachb-cpr commented 4 years ago

I'll do my best, but I don't have access to a camera right now; I did as much testing as I could before one of our integrators took the camera away to install it on a customer's robot. I'm working on getting longer-term access to a camera to do some more thorough work on this driver soon though.