Fixed the EdgeFlow Divergence computation Sign Error

[TitusBraber]

When computing flow and divergence with Edgeflow, there is a comment saying:

/* Save Resulting flow in results

• Warning: The flow detected here is different in sign
• and size, therefore this will be divided with
• the same subpixel factor and -1 to make it on par with
• the LK algorithm of t opticalflow_calculator.c
• */
  Next the flow vectors are multiplied by -1. However, the divergence wasn't multiplied by -1, a quick test when flying up and down and computing divergence with both LK and EF at the same time confirmed there was a sign difference.
  This pull request fixes that.

@kirkscheper This is the pull request I was referring to.

[kirkscheper]

@etitus did you check if any other module uses this output and would require the sign to be changed downstream?

[TitusBraber]

@kirkscheper
It is kind of impossible that other modules would use that output, unless they actually edited master without making a pull request. Let me explain:

opticflow_calc_frame computes opticflow_result_t differently depending on the LK/EF flag.
The struct contains among others:
float div_size;
float divergence;

The divergence that is calculated in EF, of which I requested the sign change, is the float divergence. The div_size is set to 0.0f hardcoded.

Higher up in opticflow_module.c the result structure is being send over the ABI. However, it only sends div_size, so in case of EF only the hardcoded 0.0f, and not the actual computed divergence.

    AbiSendMsgOPTICAL_FLOW(OPTICFLOW_SEND_ABI_ID, now_ts,
                           opticflow_result.flow_x,
                           opticflow_result.flow_y,
                           opticflow_result.flow_der_x,
                           opticflow_result.flow_der_y,
                           opticflow_result.noise_measurement,
                           opticflow_result.div_size,
                           opticflow_state.agl);

This itself is kinda not as it should be in my opinion. I discussed it with @OpenUAS, but decided that for now I would change all kinds of things in the ABI, I'll save that for later/after discussing with you and Kimberly and others. t

So summarizing, the sign change can't have influence on other modules unless people changed what is put into the abi message in their own forks, which we cannot oversee.

[kirkscheper]

I am not sure that 'it is kind of impossible that other modules would use that output' as it is currently used in the 'paparazzi/sw/airborne/modules/computer_vision/opticflow_module.c'. There it is only a telemetry message but we need to make sure that there are no side effects for others from this change.

@knmcguire or I will look at this more when I get back. It seems like you are the only one actively using this at the moment so is not a priority to pull into master asap.

[kirkscheper]

@etitus can you add a comment to the definition of the opticflow_result_t defining the direction of the divergence? Usually, divergence is positive with motion in the direction of the camera view but I know that different people use different definitions so make it explicit. Also add an axis definition for the x and y flow.

[gautierhattenberger]

should we merge this or not ?

[knmcguire]

It is just a tiny change, however we do want @etitus to add axis definitions to the module to indicate the camera coordinate system used for for optical flow. If anybody else uses the module, still assuming the old coordinate system, a drone might crash because of it. I know that divergence is used by some people for height estimation and control, and they might use an external variable instead of the abi message.

@etitus, could you add the definitions as soon as possible? To prevent pull request from pilling up .

[TitusBraber]

I'll try to do that today! What would be the best place to define this? I will put a comment before the line regardless to explain why it is happening, but in the struct definition might be nice to also have it defined, so in inter_thread_data.h
That will automatically also define the Lucas Kanade flow direction which is also not there.

[knmcguire]

you could look at the stereocam.c module as an example maybe?

[OpenUAS]

After graduation @etitus promised us to clean up and finish the PR, so that will be in the magic "soon finished" region ;-) of timeframes. We will support him wherever we can there.

[TitusBraber]

Will add a Pull Request after #2118 has been implemented

[TitusBraber]

Updated the PR with the definitions of positive directions.
Furthermore I put the value of divergence in div_size in the result structure, to avoid encountering a hardcoded 0.0 when flying on div_size and switching from LK to EF. Alternatively I can add divergence to the abi structure, but as that has a higher impact on PPRZ I opted for this solution.

Finally I removed the noise_measurement that was set to 0.0, as it was set to 0.2 a few lines later.