-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Fps permutations test #12335
Fps permutations test #12335
Conversation
@AviaAv always check if your test indeed run in all required OS. |
Please rebase development and run CI manually with nightly context and verify the test run and pass on all supported OS's |
f"{sensor_profile_dict[sensor_key][0].stream_name()} is {expected_fps_dict[sensor_key]}" | ||
f" on {get_resolution(sensor_profile_dict[sensor_key][0])}\n") | ||
s = s.replace("on (0, 0)", "") # remove no resolution for Motion Module profiles | ||
s += "***************" |
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.
Remove. Might be helpful for debug but we don't want this in the final logs
funcs_dict = generate_functions(sensor_fps_dict) | ||
|
||
for key in sensor_profiles_dict: | ||
sensor = key |
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.
Just use for sensor in ...
sensor_fps_dict[key] /= TIME_TO_COUNT_FRAMES | ||
# now for each sensor we have the average fps received | ||
|
||
sensor = key |
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.
Same
############################################# | ||
# ---------- Core Test Functions ---------- # | ||
############################################# | ||
def check_fps(expected_fps, fps_measured): |
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.
Rename check_fps_dict
. Also it is better to have measured fps as the first argument for consistency with our other check functions use (and also your own check_fps_pair).
# to avoid having a long run time, we don't choose the same stream more than once | ||
if p.stream_type() not in stream_types_added: | ||
if p.stream_type() == rs.stream.infrared: | ||
if p.stream_index() != 1: |
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.
Style comment (decide if you want to change) - this is very indented you can use and
instead of the last if
, or better yet, move the last 3 if
s to a function should_add_profile
if p.stream_type() not in stream_types_added: | ||
if p.stream_type() == rs.stream.infrared: | ||
if p.stream_index() != 1: | ||
continue # on some devices, using Infrared 1 seems to have better fps than IR/IR2 |
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.
The comment is not clear - which models and why...
According to our talk with Idan you can use validation KPIs are for depth+IR1 not IR2
ca25baf
to
721218f
Compare
e159742
to
e35a008
Compare
Tracked on [LRS-909]