Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

EKFGSF_yaw::updateEKF() R_prev copy safely #775

Merged
merged 3 commits into from Mar 11, 2020

Conversation

dagar
Copy link
Member

@dagar dagar commented Mar 10, 2020

Clang-tidy didn't like this. http://ci.px4.io:8080/blue/organizations/jenkins/PX4%2FFirmware/detail/PR-14301/4/pipeline

src/lib/ecl/EKF/EKFGSF_yaw.cpp:467:2: error: undefined behavior, source object type 'matrix::Dcmf' (aka 'Dcm<float>') is not TriviallyCopyable [bugprone-undefined-memory-manipulation,-warnings-as-errors]

         memcpy(&R_prev, &_ahrs_ekf_gsf[model_index].R, sizeof(R_prev));

Copy link
Contributor

@jkflying jkflying left a comment

Choose a reason for hiding this comment

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

I believe Paul did this to reduce the number of copied data in GSF, since this happens 5x as much. Technically this could actually be reduced down to just 3 values, not a 2x3, since the second 3 get updated in-place, you just need the original value of the first 3 to not alias the multiplication with its own results.

@dagar
Copy link
Member Author

dagar commented Mar 10, 2020

That's also fine. Updated.

Copy link
Contributor

@jkflying jkflying left a comment

Choose a reason for hiding this comment

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

Looks good, assuming the change indicator is fixed after a rebase.

@bresch bresch merged commit 580118e into master Mar 11, 2020
@bresch bresch deleted the pr-ekf_ekfgsf_yaw_minor_fix branch March 11, 2020 14:49
@priseborough priseborough added the non-functional Used for code changes that do not change functionality label Jun 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
ekf non-functional Used for code changes that do not change functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants