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

must turn feature on before setting mode to auto / manual #15

Closed
bricerebsamen opened this issue Aug 16, 2013 · 20 comments
Closed

must turn feature on before setting mode to auto / manual #15

bricerebsamen opened this issue Aug 16, 2013 · 20 comments
Assignees
Labels
bug
Milestone

Comments

@bricerebsamen
Copy link

@bricerebsamen bricerebsamen commented Aug 16, 2013

On my ladybug3, I found that I had to turn on a feature before attempting to change the mode to auto or manual (and possibly one push). See below for my quick solution.

bool Features::setMode(dc1394feature_info_t *finfo, dc1394feature_mode_t mode)
{
dc1394feature_t feature = finfo->id;
if (hasMode(finfo, mode))
{
ROS_DEBUG_STREAM("setting feature " << featureName(feature)
<< " mode to " << modeName(mode));
if (finfo->on_off_capable &&
(mode==DC1394_FEATURE_MODE_MANUAL || mode==DC1394_FEATURE_MODE_AUTO
|| mode==DC1394_FEATURE_MODE_ONE_PUSH_AUTO) )
{
if (DC1394_SUCCESS !=
dc1394_feature_set_power(camera_, feature, DC1394_ON))
{
ROS_WARN_STREAM("failed to turn feature " << featureName(feature)
<< " on prior to setting it to mode "
<< modeName(mode));
return false;
}
}
if (DC1394_SUCCESS !=
dc1394_feature_set_mode(camera_, feature, mode))
{
ROS_WARN_STREAM("failed to set feature " << featureName(feature)
<< " mode to " << modeName(mode));
return false;
}
}
else
{
// device does not support this mode for this feature
ROS_DEBUG_STREAM("no " << modeName(mode)
<< " mode for feature " << featureName(feature));
return false;
}
return true;
}

@jack-oquin

This comment has been minimized.

Copy link
Member

@jack-oquin jack-oquin commented Aug 16, 2013

How are you attempting to set the feature?

There are known bugs in rqt_reconfigure (ros-visualization/rqt_common_plugins#107).

@bricerebsamen

This comment has been minimized.

Copy link
Author

@bricerebsamen bricerebsamen commented Aug 16, 2013

I'm using rosparam for the initial values, then dynamic_reconfigure (as a
standalone). I don't use rqt
Plus, I'm still on Fuerte...

On Thu, Aug 15, 2013 at 6:46 PM, jack-oquin notifications@github.comwrote:

How are you attempting to set the feature?

There are known bugs in rqt_reonfigure (
ros-visualization/rqt_common_plugins#107ros-visualization/rqt_common_plugins#107
).


Reply to this email directly or view it on GitHubhttps://github.com//issues/15#issuecomment-22743148
.

@jack-oquin

This comment has been minimized.

Copy link
Member

@jack-oquin jack-oquin commented Aug 16, 2013

I don't think the current camera1394 master branch works on Fuerte. Are you running the fuerte-devel branch from source? Or installing the Ubuntu Debian?

To fix this, I need to know how to reproduce the error.

@bricerebsamen

This comment has been minimized.

Copy link
Author

@bricerebsamen bricerebsamen commented Aug 16, 2013

Actually I backported the pan and trigger features. From the master branch I created patches for those features and applied them to the fuerte branch. If this is of interest I could make this available in github. Anyway, I saw that the setMode function has the same problem in both fuerte and master.

Conceptually (from the IIDC specs), does an on/off capable feature has to be turned on before attempting to set it to auto or manual? Initially I thought that changing the mode from off to auto would turn it on at the same time, but from what I experienced with my camera yesterday it is not the case. The modification I sent you solved the issue.

@jack-oquin

This comment has been minimized.

Copy link
Member

@jack-oquin jack-oquin commented Aug 16, 2013

You are right that setMode() has not changed in the last year or two.

But, there are thousands of different 1394 cameras. No two behave exactly alike. For a start, it would help to know the make and model of your camera which exhibits this problem. Do you have any other cameras with similar behavior? What are they? Do you have other cameras with different behavior?

Then, I need a step-by-step procedure that reproduces the bug. What did you do? What happened? What should have happened?

With that, I can try to reproduce this problem with some of my test cameras.

If it turns out that this problem only occurs on a few specific cameras, but the fix works safely for other cameras, I will apply your fix to the current master. Maybe we can create a unit test case that demonstrates this bug.

Since no one else has reported this problem in the last year or so, I will probably not release the fix to Fuerte or Groovy. That avoids destabilizing anyone who was already doing fine with those releases.

Since the current master also works on Groovy, anyone needing the change there can compile from sources. If you want to maintain a Fuerte back-port with this fix, someone might find that helpful. It's hard to guess without knowing how widespread the problem is.

@bricerebsamen

This comment has been minimized.

Copy link
Author

@bricerebsamen bricerebsamen commented Aug 16, 2013

I am only testing with the ladybug3 from point grey. Here is how to reproduce:

  • pick a feature that has on/off capability, say exposure
  • turn it off
  • then set it to manual

what happens: it refuses to go to manual and stays off. Same with auto mode.

my patch solves the problem. But I understand what you say.

On my side, I backported all recent changes to fuerte and fixed this problem. I have 2 patches for that, that I can send to anyone is interested. At the same time, I think anyone who is interested can easily do it themselves ... (diff + patch) ;)

@jack-oquin

This comment has been minimized.

Copy link
Member

@jack-oquin jack-oquin commented Aug 16, 2013

Sorry. You had mentioned the ladybug3 up at the top and I forgot. Are you using a separate nodelet to separate the different images? Is it available on-line?

I'll try that test with some of my cameras. I am pretty sure they worked originally, but that was some time ago. It should be possible and worthwhile to write some unit tests for it.

What happens if you first turn it back on, before setting it to manual? That works OK? It looks like your code does something similar.

@bricerebsamen

This comment has been minimized.

Copy link
Author

@bricerebsamen bricerebsamen commented Aug 16, 2013

How to turn it ON? I only have options for Off, Query, Auto, Manual, OnePush, None...

To separate the images I wrote my own code. You have to separate them, decompress them, and combine the 4 color images to create the Bayer pattern. I can make it available. I need some more time to polish it... but if you want it quickly to do some test I can send it straight away, as is.

@jack-oquin

This comment has been minimized.

Copy link
Member

@jack-oquin jack-oquin commented Aug 16, 2013

Yeah. I forgot there was no On setting. It's been a while since I worked on this stuff. As long as your fix works with other cameras, that seems like a good solution.

I don't need your splitter nodelet, because I don't have any bumblebee cameras. I asked because others would probably be interested. If it works well and you are willing to maintain it, we could add it to ros-drivers.

@bricerebsamen

This comment has been minimized.

Copy link
Author

@bricerebsamen bricerebsamen commented Aug 16, 2013

Fine, I can share it and maintain it, that's a small amount of code and it's straightforward.

However, as I said, I'm still under Fuerte and rosbuild, so I'll need help to catkinize it and test it under groovy and hydro.

@chadrockey

This comment has been minimized.

Copy link
Member

@chadrockey chadrockey commented Aug 16, 2013

If you're having issues with specific features of PointGrey cameras, feel free to try pointgrey_camera_library. It's based around PointGrey's FlyCap library.

https://github.com/NREC/camera-drivers/tree/master/pointgrey_camera_driver

@ghost ghost assigned jack-oquin Aug 17, 2013
@jack-oquin

This comment has been minimized.

Copy link
Member

@jack-oquin jack-oquin commented Aug 17, 2013

Fine, I can share it and maintain it, that's a small amount of code and it's straightforward.

However, as I said, I'm still under Fuerte and rosbuild, so I'll need help to catkinize it and test it under groovy and hydro.

I've had more practice than I ever wanted this year at converting rosbuild packages to catkin. If you create a rosbuild package for ladybug cameras, I'll catkinize it for you and help you release it.

@jack-oquin

This comment has been minimized.

Copy link
Member

@jack-oquin jack-oquin commented Aug 17, 2013

Brice,

Back to the original error you reported here: I tried it with my usual test cameras, but discovered that none of them provide any features that are On/Off capable. I'll see if I can find some other cameras to test it with, but it looks like the bug you found is probably quite general and has been in the code all along.

I can only guess that most other cameras don't provide the feature power-off capability either, or your problem should have been uncovered long ago.

I'll commit a fix some time soon. I may need to ask you to regression-test it for me.

@jack-oquin

This comment has been minimized.

Copy link
Member

@jack-oquin jack-oquin commented Aug 18, 2013

@bricerebsamen: The fix I committed is somewhat different from yours. It works with my cameras, but that does not prove much. At least, it does not break cameras that do not require it.

So, it needs testing with your camera.

That requires a minimal Groovy or Hydro install. You can install ros-hydro-camera1394 to get all the dependencies and then check out the master branch into a catkin workspace. I can provide detailed steps, if you like. The biggest problem is the rqt_reconfigure bug mentioned above. I used the rosrun dynamic_reconfigure dynparam command as a work-around. That's a hassle, but it works.

A code review would also be useful if you have time.

Thanks for reporting this problem and figuring out how to fix it.

@jack-oquin

This comment has been minimized.

Copy link
Member

@jack-oquin jack-oquin commented Aug 24, 2013

Brice: have you had a chance to test this yet?

I hope to make a new release in the next week or so, and don't want to release a broken version of your fix.

@bricerebsamen

This comment has been minimized.

Copy link
Author

@bricerebsamen bricerebsamen commented Aug 29, 2013

sorry for taking so long. I started working on it. I started catkinizing some of my packages so that I could do the frame parsing to check that the result is right, but it's taking me some time as I am not familiar with Catkin yet. I can't say I'm enjoying the process either... But I'm on it, I just need a bit more time.

@jack-oquin

This comment has been minimized.

Copy link
Member

@jack-oquin jack-oquin commented Aug 29, 2013

That's OK, the triggering enhancement (#19) is not ready to release yet, either.

I was not expecting you to need to do a lot of work to run the test.

Can you install ros-hydro-camera1394, source /opt/ros/hydro/setup.bash, build a catkin workspace with camera1394 master source, connect to your camera, and see what happens when turning some feature off and on? I can provide step-by-step instructions, if that helps.

@bricerebsamen

This comment has been minimized.

Copy link
Author

@bricerebsamen bricerebsamen commented Sep 3, 2013

OK I took a simpler approach, after all I don't need to see the images... I did the following experiment with both 7481d14 (pre my fix) and 24d71be (the latest today).

I started loaded a set of parameters that I know are working well with my camera. In particular auto_exposure set to AUTO.
I started the camera1394_node
I used dynparam to check that auto_exposure was set to AUTO
I used dynparam to set auto_exposure to OFF and then back to AUTO
I used dynparam to get the value of auto_exposure

With rev 7481d14 auto_exposure was 0 (OFF), and with rev 24d71be it was 2 (AUTO).

So I believe the fix introduced in 24d71be solved the problem.

@jack-oquin

This comment has been minimized.

Copy link
Member

@jack-oquin jack-oquin commented Sep 3, 2013

That sounds like a good test. Thanks!

I'll release this fix to Hydro as soon as the Trigger enhancements are also ready.

Meanwhile, thank you very much for identifying this problem and coming up with a solution that works.

@bgromov

This comment has been minimized.

Copy link
Contributor

@bgromov bgromov commented Sep 4, 2013

@bricerebsamen, @jack-oquin

Conceptually (from the IIDC specs), does an on/off capable feature has to be turned on before attempting to set it to auto or manual? Initially I thought that changing the mode from off to auto would turn it on at the same time, but from what I experienced with my camera yesterday it is not the case.

According to IIDC v1.31 (p.29) if the feature is OFF then all its settings will be _read only_ (see fields ON_OFF in second table of section 4.7 of IIDC specs).

Same applies to trigger feature (see comment on #14 (comment)).

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