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

[WIP] Camera capture driver #10295

Closed
wants to merge 22 commits into from
Closed

[WIP] Camera capture driver #10295

wants to merge 22 commits into from

Conversation

DanielePettenuzzo
Copy link
Contributor

@DanielePettenuzzo DanielePettenuzzo commented Aug 21, 2018

This PR adds the camera capture driver. This can be used to receive a feedback from a camera after it is triggered (eg. sony hot shoe feedback). In this case we don't publish the camera_trigger message in the camera_trigger driver when we trigger the camera, but we publish when we receive the feedback from the camera. When geotagging images this will increase the presision in the geotag.

This can be also used to sync CV cameras with the IMU data for VIO or similar applications.

Currently only channel 5 on the FMU PWM outputs can be used for this driver. I still need to check on which outputs we can actually run this.

Some more tests will be done in the next days.

Params for testing:
CAM_CAP_FBACK = 1 (enable camera capture)

CAM_CAP_MODE =
0 (get edge timestamp)
1 (get pulse midpoint timestamp for active high triggers)
2 (get pulse midpoint timestamp for active low triggers)

CAM_CAP_EDGE =
0 (trigger on falling edge)
1 (trigger on rising edge)

CAM_CAP_DELAY --> delay between image integration start and strobe firing (default to 0)

@DanielePettenuzzo
Copy link
Contributor Author

FYI @mhkabir @bresch

@mhkabir mhkabir self-requested a review August 22, 2018 00:05
@hamishwillee
Copy link
Contributor

  1. Can we have in-module docs like these ones: https://github.com/PX4/Firmware/blob/master/src/drivers/batt_smbus/batt_smbus_main.cpp#L248

That autogenerates docs like this: https://dev.px4.io/en/middleware/modules_driver.html

  1. More generally, currently we have this doc on the camera (which needs serious updates).

Can you provide some more overview on this?

  1. Why is this needed - what can you do with it you couldn't do before
  2. What devices can it be used with.
  3. Is this present in firmware by default
  4. Is any special hardware needed to connect it? Example of use? Images>
  5. You say In this case we don't publish the camera_trigger message in the camera_trigger driver when we trigger the camera, but we publish when we receive the feedback from the camera. - so the camera trigger knows about this component and won't automatically publish.

Essentially I'm after what user-facing docs we need so that someone who wants to use this feature can find it and knows what to plug in where.

platforms__common
EXTERNAL
)
# vim: set noet ft=cmake fenc=utf-8 ff=unix :
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

delete

@@ -0,0 +1,313 @@
/****************************************************************************
*
* Copyright (c) 2017 PX4 Development Team. All rights reserved.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

year

@@ -0,0 +1,136 @@
/****************************************************************************
*
* Copyright (c) 2017 PX4 Development Team. All rights reserved.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

year

@dagar dagar requested review from dagar and davids5 August 29, 2018 15:21
Copy link
Member

@davids5 davids5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See in-line comments. We need to not do work on the timer isr. (Capture call back) Use the worker queue.
Assume the FMU owns the resources. (things listed as up_xxxxxxx should not be directly accessed by high level drivers) Use the IOCTL and the modes to the FMU to achieve what is needed. If it needs to be extended for V5 add that to the px4fmu.

param_get(_p_camera_capture_edge, &_camera_capture_edge);

if (_camera_capture_feedback) {
struct camera_trigger_s trigger = {};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is publishing a null message. Is that of consequence? Once this get fixed in uOrb this can use the new API. Until then using advertise on first publication (not on an ISR) will work.

}

void
CameraCapture::capture_callback(uint32_t chan_index,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming this is the ISR call back. Long service is not recommended on a timer call back. So off load the values to a queue or buffer. Then scheduled HP work to read the buffer and do the logic and publication.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The also facilitates the lazy advertise as it can be done in the context of a work thread and not an ISR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed a fix for this.. Now I just push to a buffer in the ISR callback and then schedule the rest in the work queue. On average it is 2 or 3 micro seconds faster than before.

then
if camera_capture start
then
set FMU_MODE pwm4
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Work should be done on the px4fmu to support the mode needed on fmu V5.

https://github.com/PX4/Firmware/blob/master/src/drivers/px4fmu/fmu.cpp#L3048-L3050

The script should select that mode.

}

void
CameraCapture::set_capture_control(bool enabled)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The coordination is done using a mode command to the fmu. as seen here https://github.com/PX4/Firmware/blob/master/src/drivers/px4fmu/fmu.cpp#L3048-L3050

You will need to expand the fmu to support V5 captures.

Then use the IOCTL https://github.com/PX4/Firmware/blob/master/src/drivers/px4fmu/fmu.cpp#L2314

Here is an example https://github.com/PX4/Firmware/blob/master/src/drivers/px4fmu/fmu.cpp#L2625-L2650

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this an opportunity to move away from the fmu dependency entirely?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could be. But one would have to move all the code that manages the pwm and capture and all the pins to another module. There needs to be a single point of control and configuration. As it stands it was the FMU

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fwiw, FMU input capture is currently broken and crashes the board if the IOCTL is used. I filed an issue sometime back.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mhkabir It was fixed at one point perhaps not upstreamed. Did you test on current master?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried this quickly today but was not working. This branch is about a week behind master. @davids5 where can I see if this was fixed?

@@ -126,7 +126,7 @@ static void input_capture_chan_handler(void *context, const io_timers_t *timer,
if (channel_handlers[chan_index].callback) {
channel_handlers[chan_index].callback(channel_handlers[chan_index].context, chan_index,
channel_stats[chan_index].last_time,
channel_stats[chan_index].last_edge, overflow);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is correct as is. It is not a count is it the state now.

@DanielePettenuzzo
Copy link
Contributor Author

@hamishwillee As soon as I'm done cleaning this up I will also look at the docs. I also agree that there is some stuff missing on the dev guide.

@jasonbeach
Copy link

I am trying to test this on a pixracer but am having an issue. It seems that when you enable the trigger capture, triggers stop being sent out by the pixracer. My thought was to send a trigger out on pin 6 of the pixracer and then wait for feedback on pin 5. I guess I can see if I can configure the camera so it doesn't need to be triggered, but still sends out a pulse when it starts (or finishes) taking an image.

In an attempt to do this I set the TRIG_PINS parameter to 6 (so 5 could be used for the capture feedback) however on rebooting the autopilot made an angry sounding beep (5 long beeps). It would arm when requested to, but then wouldn't do anything. Setting TRIG_PINS back to 56 makes the angry beeps go away and allows the vehicle to takeoff as expected. What's the intended Trigger config when using the capture feedback?

@mhkabir
Copy link
Member

mhkabir commented Sep 21, 2018

Triggers stop being sent out by the pixracer. My thought was to send a trigger out on pin 6 of the pixracer and then wait for feedback on pin 5.

Doesn't work. 5 and 6 have to have the same functionality due to timer allocation.

@davids5
Copy link
Member

davids5 commented Sep 25, 2018

@DanielePettenuzzo , @mhkabir - Here PX4-Works#1 is a PR based against your PR (I do not have write access to Auterion)

60d0a08 was cherry-picked from my PR #10567 (and merge resolved)
PX4-Works@57dbc0b is the code using IOCTL

@tubeme
Copy link

tubeme commented Sep 28, 2018

@hamishwillee

Can you provide some more overview on this?

Why is this needed - what can you do with it you couldn't do before.

If we want a precision mapping calibration method in Pix4D and other mapping solutions we depend on the trigger timing for our precision. IE at a geolocated spot we have coordinates, altitude, angles, precision and time we were at that spot. If we need a centimiter precision in the mapping method and we are flying with 14 m/s in the air we have a difference in time when the PX4 commanded trigger and when the actual cammera trigger makes the shot. So this difference in space could be from dozen of centimeters to a meter depending on the delay of the camera trigger. So by providing the horseshoe feedback from the camera we are able to get the real timing of the image, thus placing it in the right place in time to achieve cm precision mapping.

What devices can it be used with.

As far as I understand it is Sony for now. But it could not be that hard to extend the code for other cameras. Sony is the most common high res mapping camera used now.

Is this present in firmware by default
Should be. With some nice interface in the QGC to setup on which channel is the horseshoe connected.

Is any special hardware needed to connect it? Example of use? Images>
I'm not aware of any special hardware. The writer of the driver could explain how does he connect to the camera.

You say In this case we don't publish the camera_trigger message in the camera_trigger driver when we trigger the camera, but we publish when we receive the feedback from the camera. - so the camera trigger knows about this component and won't automatically publish.

Yes indeed. If we have horseshoe connected we have to get its trigger time stamp instead of the commanded one in the PX4.

I think this pretty much explains it. @DanielePettenuzzo should add more info.

@hamishwillee
Copy link
Contributor

Thanks @tubeme @DanielePettenuzzo
Just one point:

You say In this case we don't publish the camera_trigger message in the camera_trigger driver when we trigger the camera, but we publish when we receive the feedback from the camera. - so the camera trigger knows about this component and won't automatically publish?

Yes indeed. If we have horseshoe connected we have to get its trigger time stamp instead of the commanded one in the PX4.

I understand the concept, but can one of you point me to where in code it is that stops the normal camera driver from triggering? ie there is nothing to stop it publishing from what I can see. Perhaps when this is enabled the normal driver is somehow disabled?

@DanielePettenuzzo
Copy link
Contributor Author

@hamishwillee currently both the trigger and the capture drivers are always publishing and then the camera_feedback module decides which one to use based on the CAM_CAP_FBACK param. The one that is used is then copied into the camera_capture message together with gps and attitude data. https://github.com/PX4/Firmware/blob/2b3b0ebadb865c8e5964c05e24b0f2f04528c2bb/src/modules/camera_feedback/camera_feedback.cpp#L125
The main reason is so I can log both the trigger output and the feedback, so that we can see from the log if the camera has issues (more triggers than feedbacks). For the mavlink message I still need to fix this because it is currently still sending the camera_trigger output.

@tubeme
Copy link

tubeme commented Oct 5, 2018

Hi @DanielePettenuzzo can you share some notes on how do you connect the hotsshoe to the PX4? Where did you get the connector for the Camera?

@DanielePettenuzzo
Copy link
Contributor Author

DanielePettenuzzo commented Oct 10, 2018

Hi @tubeme,
I will document this soon in the user/dev guide. The following is what is already documented: https://docs.px4.io/en/peripherals/camera.html.

The hardware I currently use is the following:

I use pins 1 and 2 on the fmu port for triggering (#Map2 needs two pwm inputs) together with 5 volts and ground. The documentation says to use a 5V BEC but I got it to work also by providing power from one of the pixhawk's 5V pins. Then I connect the #Map2 to the camera using the map cable listed above. For the feedback I plug the #SYNC into the hotshoe port on the camera. The signal on the #SYNC goes to pin 6 on the fmu pwm port that will be used for the input capture. Also the #SYNC needs 5V and ground from a 5V BEC. Also in this case I managed to get it to work using 5V from the Pixhawk.

To enable triggering I go on the camera tab in qgc and enable triggering by choosing a control mode and the interface. The mode can be what you need for your application (either time or distance based). The interface has to be "Seagull Map2 (over PWM)". Then you can choose the two pins you want to use for triggering: either 1 and 2 or 3 and 4. We still have some limitations on which pins can be used so you will have to control motors and servos from the IO port for now.

For the camera capture instead there is no GUI on qgc so you will have to configure a few parameters in qgc.
CAM_CAP_FBACK = 1 (enable camera capture)

I still need to push a few changes to make all of this work better for logging and for mavlink. Anyway now the camera_trigger driver publishes on the camera_trigger topic, the camera_capture on the camera_trigger_feedback topic and the camera_feedback module takes one of the two depending if the capture is enabled or not and synchronizes the timestamp to the attitude and gps topics. This will end up in the camera_capture topic that is used by qgc for geotagging.

At the moment just the camera_trigger and the camera_capture topics are logged and also sent via mavlink. With the changes I will push also mavlink can choose what trigger timestamp to use and the logger can log all topics if necessary.

I hope this can get you started. If you have any questions let me know. The PX4 documentation will be soon updated with all this info.

@DanielePettenuzzo
Copy link
Contributor Author

PX4-Works#1 was merged

@DanielePettenuzzo
Copy link
Contributor Author

Rebased

@@ -630,6 +632,23 @@ PX4FMU::set_mode(Mode mode)

break;

#if defined(BOARD_HAS_CAPTURE)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@davids5 I added this mode only for the camera_capture. Do you think it's ok?

@DanielePettenuzzo
Copy link
Contributor Author

@davids5 @dagar With my latest commit I can now run the camera_capture on fmu pin 6 and still be able to control the motors with 400 Hz pwms on pins 1 to 4. The problem that I still have is if we want to have both camera_trigger and camera_capture running at the same time. Camera_trigger can only run on pins 1 to 6 on the fmu. But in the current setup of the camera_capture takes the first 6 pins for the fmu. The ideal situation would be to have camera_capture on the CAP/ADC pins so it wouldn't interfere with the fmu. Another way to solve it would be to have also the timers on pins 7 and 8 configured so the fmu can take 1-6 (1-4 for motors, 5-6 for the capture) and the trigger driver can use 7-8.

Moreover if I switch to use the IO for motor control the camera_capture doesn't work anymore because it depends on the fmu.

Copy link
Member

@davids5 davids5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DanielePettenuzzo It would be best if the FMU changes are a separate commit. Please check the module help. I think pwm4cap2 may not be listed

@DanielePettenuzzo
Copy link
Contributor Author

I divided the fmu commit from the camera_capture commit and fixed a bug where I was always pulling the older capture from the trig_buffer. Now I replaced this buffer with a simple public structure in the driver and this solves my issue.

@mhkabir
Copy link
Member

mhkabir commented Dec 29, 2018

@DanielePettenuzzo Would you have some time to wrap this up and address the minor renaming comments from above?

mhkabir and others added 22 commits January 29, 2019 22:27
… camera feedback module uses CAM_CAP_FBACK param to choose between the two.
   This extends the  Capture support for FMU
   CHAN 5 and 6.
@hamishwillee
Copy link
Contributor

@DanielePettenuzzo Docs?

@jasonbeach
Copy link

@hamishwillee Was this slated for v1.9.0? I thought it was but it seems that milestone was just removed. if not what's the best way to build a custom version of the firmware with this?

@hamishwillee
Copy link
Contributor

Was this slated for v1.9.0? I thought it was but it seems that milestone was just removed. if not what's the best way to build a custom version of the firmware with this?

@jasonbeach I have no idea, hence the question for @DanielePettenuzzo (?). This needs docs before it goes into a release.

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.

8 participants