-
Notifications
You must be signed in to change notification settings - Fork 81
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
Feature/playback get imu sample #150
Feature/playback get imu sample #150
Conversation
Sorry, I did not notice this PR until today! Thanks for the contribution, this looks well done. @shagren it seems perfect but could you test? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great!
Just two small changes for better quality
@@ -178,6 +178,12 @@ def get_previouse_capture(self): | |||
thread_safe=self.thread_safe, | |||
) | |||
|
|||
def get_next_imu_sample(self) -> Optional["ImuSample"]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add
from .pyk4a import ImuSample
before
from .results import Result, StreamResult
pyk4a/pyk4a.cpp
Outdated
imu_sample.acc_sample.xyz.x, imu_sample.acc_sample.xyz.y, imu_sample.acc_sample.xyz.z, | ||
"acc_timestamp", imu_sample.acc_timestamp_usec, "gyro_sample", imu_sample.gyro_sample.xyz.x, | ||
imu_sample.gyro_sample.xyz.y, imu_sample.gyro_sample.xyz.z, "gyro_timestamp", | ||
imu_sample.gyro_timestamp_usec); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
imu_sample.acc_sample.xyz.x, imu_sample.acc_sample.xyz.y, imu_sample.acc_sample.xyz.z, | |
"acc_timestamp", imu_sample.acc_timestamp_usec, "gyro_sample", imu_sample.gyro_sample.xyz.x, | |
imu_sample.gyro_sample.xyz.y, imu_sample.gyro_sample.xyz.z, "gyro_timestamp", | |
imu_sample.gyro_timestamp_usec); | |
imu_sample.acc_sample.xyz.x, imu_sample.acc_sample.xyz.y, imu_sample.acc_sample.xyz.z, | |
"acc_timestamp", imu_sample.acc_timestamp_usec, "gyro_sample", imu_sample.gyro_sample.xyz.x, | |
imu_sample.gyro_sample.xyz.y, imu_sample.gyro_sample.xyz.z, "gyro_timestamp", | |
imu_sample.gyro_timestamp_usec); |
Just remove 2 spaces on these lines
…port in playback.py
Sorry, now I didn't see your comments for a long time. Hopefully this can be closed soon now. :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Codecov Report
@@ Coverage Diff @@
## develop #150 +/- ##
===========================================
+ Coverage 81.23% 81.38% +0.14%
===========================================
Files 12 12
Lines 762 768 +6
===========================================
+ Hits 619 625 +6
Misses 143 143
Continue to review full report at Codecov.
|
* Feature/playback get imu sample (#150) * feat(playback-imu-sample): add "get_next_imu_sample" to playback * fix(playback-imu-sample): PR changes, add correct indentation, add import in playback.py * add info related to conda and DLL not found error * update black version (fix click bug) (#168) * update black version (fix click bug) * black reformat * fix typo Co-authored-by: Louis-Philippe Asselin <lpasselin@users.noreply.github.com> Co-authored-by: annStein <43335656+annStein@users.noreply.github.com>
For playback.py I added the method to get the next imu sample.