-
Notifications
You must be signed in to change notification settings - Fork 4
fix and test software triggering #34
fix and test software triggering #34
Conversation
Fixes an unchecked failure in `acquire_map_read()` caused when a caller fails to call `acquire_unmap_read()` at the right time. For example: ```c acquire_map_read(runtime,0,&beg,&end); // When end>beg acquire_map_read(runtime,0,beg,end); // Should return AcquireStatus_Error ``` whereas ```c acquire_map_read(runtime,0,&beg,&end); // When end>beg acquire_unmap_read(runtime,0,&beg,&end); acquire_map_read(runtime,0,beg,end); // Should return AcquireStatus_Ok ``` acquire-project/acquire-driver-common#34 depends on this
…acquire-driver-common into 32-add-test-for-software-trigger
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.
I don't have enough knowledge/context to approve or request changes here, but hopefully my comments and questions are useful.
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.
A few things I'd like to see changed, but no real showstoppers. Could merge as is.
while (end == beg && is_running(runtime) && | ||
clock_toc_ms(&t0) < test_timeout_ms) { | ||
// delay so loop isn't busy polling | ||
clock_sleep_ms(0, 10); | ||
OK(acquire_map_read(runtime, 0, &beg, &end)); | ||
} | ||
|
||
if (end == beg && !is_running(runtime)) | ||
break; |
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.
@andy-sweet I think the test is not flaky now.
I kept the sleep in the loop just so it's not burning CPU. I had to add the additional is_running
check. Otherwise, there was a window where the acquisition would stop but it would make it to line 167 and hit the test timeout (and fail).
closes #32