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

Pose history #1737

Merged
merged 11 commits into from
Jun 20, 2016
Merged

Pose history #1737

merged 11 commits into from
Jun 20, 2016

Conversation

rmeertens
Copy link
Contributor

In #1729 @knmcguire and @flixr had the idea of creating a pose history module... So here it is!

Currently it records at 512hz, and remembers its state for two seconds (1024 measurements). Searching in the buffer is not very efficient, but good enough for now to test the optical flow.

#ifndef POSE_HISTORY_H
#define POSE_HISTORY_H

#include "state.h"
Copy link
Member

Choose a reason for hiding this comment

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

Should include "math/pprz_algebra_float.h" instead of state.h

@flixr
Copy link
Member

flixr commented Jun 17, 2016

Nice, that is a good start to test!
Some more comments (e.g. what the timestamp is... unix time, pprz time in what unit...?) would be nice ;-)
plz also fix the code style...

@flixr
Copy link
Member

flixr commented Jun 17, 2016

Also do you really want Eulers?

@rmeertens
Copy link
Contributor Author

Fixed the includes and comments.
As for Eulers: you are right, but this is what is used now by the opticflow module.

Would you like it if I also log FloatQuat in my struct timeAndRotation?

@flixr
Copy link
Member

flixr commented Jun 17, 2016

Was just wondering... no need to buffer what is not used...
But maybe better call the function get_eulers_at_timestamp then...

@rmeertens
Copy link
Contributor Author

Done!

uint32_t closestIndex = 0;

// Seach for the timestamp closes to the timestamp argument of this function.
for (index_history = 0; index_history < location_history.ring_size; index_history++) {
Copy link
Member

Choose a reason for hiding this comment

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

Would be nicer and more efficient to do a:

uint16_t i = 0;
uint32_t diff_min;
uint32_t diff_cur = abs(timestamp - location_history.ring_data[0].timestamp);
do {
    diff_min = diff_cur;
    diff_cur = abs(timestamp - location_history.ring_data[++i].timestamp);
} while(diff < diff_min);

struct FloatEulers closest_angels;
EulersCopy(closest_angels, location_history.ring_data[--i].rotation);

This will stop searching as it reaches it minimum. This is true because the ring buffer is ordered by time.

Copy link
Member

Choose a reason for hiding this comment

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

Note that this function is not thread safe(also your implementation). This should be fixed to make it useful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure if your proposed function is better. The ring buffer is indeed ordered by time, but index zero can be way after our timestamp that we want to find... this means that we might increase our diff for a while before we jump back in time and are closer to the point we want to be at.

Copy link
Member

Choose a reason for hiding this comment

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

You can start index_of_insert +1 and then do a round trip.. Note that it must be thread safe, and this new implementation is easier to make thread safe then your previous.

Copy link
Member

Choose a reason for hiding this comment

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

@fvantienen you still want something changed here?

@fvantienen
Copy link
Member

Roland you should add some mutex's to make it thread safe, but please add some #if LINUX or something around them so that this module still works for other platforms as well.

@fvantienen
Copy link
Member

Could you maybe also add the bodyRates? since I want to use that to de-rotate the image.

@rmeertens
Copy link
Contributor Author

@fvantienen these rates?

@fvantienen
Copy link
Member

Yes those 👍 Nice!


typedef struct {
uint32_t timestamp;
struct FloatEulers rotation;
Copy link
Member

Choose a reason for hiding this comment

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

should probably be rather called orientation or eulers

}

void float_rates_copy(struct FloatRates *to, struct FloatRates *from);
void float_rates_copy(struct FloatRates *to, struct FloatRates *from)
Copy link
Member

Choose a reason for hiding this comment

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

Those functions already exist:
RATES_COPY and EULERS_COPY. https://github.com/paparazzi/paparazzi/blob/master/sw/airborne/math/pprz_algebra.h

Copy link
Member

Choose a reason for hiding this comment

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

If you copy full structs of the same type, you should simply use a normal assignment...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

@rmeertens
Copy link
Contributor Author

Ok, now using the style guide (years of java programming ;) ) and fixed the assignment.

#include <sys/time.h>
#include "mcu_periph/sys_time.h"
#include "state.h"
#if LINUX
Copy link
Member

Choose a reason for hiding this comment

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

should be #ifdef __linux__

@rmeertens
Copy link
Contributor Author

Done

<module name="pose_history">
<doc>
<description>Ask this module for the pose the drone had closest to a given timestamp</description>
<define name="POSE_HISTORY_SIZE" description="Length of the pose buffer"/>
Copy link
Member

Choose a reason for hiding this comment

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

missing value...

@rmeertens
Copy link
Contributor Author

Updated conf!

@flixr flixr merged commit b543d05 into paparazzi:master Jun 20, 2016
wilcoschoneveld pushed a commit to wilcoschoneveld/paparazzi that referenced this pull request Jun 22, 2016
A modules that remembers the vehicle state for two seconds (1024 measurements). Searching in the buffer is not very efficient, but good enough for now to test the optical flow.
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.

None yet

3 participants