[RSDK-14161] Implement support for streaming#223
[RSDK-14161] Implement support for streaming#223Andrew C. Morrow (acmorrow) wants to merge 22 commits into
Conversation
JohnN193
left a comment
JohnN193
left a comment
There was a problem hiding this comment.
not really sure how users/sanding will use the uuid. streaming stuff looks good to me though
| root_["realtime_samples"] = Json::Value(Json::arrayValue); | ||
|
|
||
| output_path_ = telemetry_path / (timestamp + "_" + resource_name + "_realtime_trajectory.json"); | ||
| output_path_ = telemetry_path / (move_id_str + "_" + resource_name + "_realtime_trajectory.json"); |
There was a problem hiding this comment.
will this make it harder to pair moves with sanding data?
There was a problem hiding this comment.
It shouldn't. Before, the timestamp was being used as the unique id, basically. It got tacked onto files and logs, but it was weird, because you would see the logs advancing with real time, but the logged "unix time" was the "correlation id". Now you just get a UUID for each move: way easier to correlate, and isn't confusing in the logs where there are two "times".
That said, we should definitely alert them to the change!
There was a problem hiding this comment.
I checked in with Brad Grigsby (@bgrigsby8) and he was OK with this change.
| // from k_open, first points pending: send STREAM_START, go to k_streaming. | ||
| // from k_open, on close: go to k_buffered. | ||
| // from k_streaming, on close: go to k_draining. | ||
| // from k_buffered: send START, drain, and END in one tick, go to k_ended. | ||
| // from k_draining, once drained: send STREAM_END, go to k_ended. |
There was a problem hiding this comment.
having trouble finding it, but just to confirm does move_through_joint_positions just go through the k_buffered branch? or do we call TRAJECTORY_START somewhere else for that now?
There was a problem hiding this comment.
That's exactly right: move_through_joint_positions will call start_move_request with trajectory_samples, which will find the matching move_request constructor. That constructor will create a sample_stream that begins life in k_buffered, with all the points already there. Then the state machine knows that for k_buffered, there is no need to use the STREAM stuff and falls back to plain old TRAJECTORY_START. A little convoluted, but has the net effect that both kinds of moves arrive in the state machine the same way, and we only stream when streaming is needed. A one-shot move becomes k_buffered and goes the old way, as does a, say, degenerate stream with only one batch of points that all arrive before the worker thread tick.
| // from k_open, first points pending: send STREAM_START, go to k_streaming. | ||
| // from k_open, on close: go to k_buffered. | ||
| // from k_streaming, on close: go to k_draining. | ||
| // from k_buffered: send START, drain, and END in one tick, go to k_ended. | ||
| // from k_draining, once drained: send STREAM_END, go to k_ended. |
There was a problem hiding this comment.
That's exactly right: move_through_joint_positions will call start_move_request with trajectory_samples, which will find the matching move_request constructor. That constructor will create a sample_stream that begins life in k_buffered, with all the points already there. Then the state machine knows that for k_buffered, there is no need to use the STREAM stuff and falls back to plain old TRAJECTORY_START. A little convoluted, but has the net effect that both kinds of moves arrive in the state machine the same way, and we only stream when streaming is needed. A one-shot move becomes k_buffered and goes the old way, as does a, say, degenerate stream with only one batch of points that all arrive before the worker thread tick.
| root_["realtime_samples"] = Json::Value(Json::arrayValue); | ||
|
|
||
| output_path_ = telemetry_path / (timestamp + "_" + resource_name + "_realtime_trajectory.json"); | ||
| output_path_ = telemetry_path / (move_id_str + "_" + resource_name + "_realtime_trajectory.json"); |
There was a problem hiding this comment.
It shouldn't. Before, the timestamp was being used as the unique id, basically. It got tacked onto files and logs, but it was weird, because you would see the logs advancing with real time, but the logged "unix time" was the "correlation id". Now you just get a UUID for each move: way easier to correlate, and isn't confusing in the logs where there are two "times".
That said, we should definitely alert them to the change!
2 participants
No description provided.