add time sync to host. #31
Conversation
There was a problem hiding this comment.
i need to update the drake sha at after the drake part merges.
+@sammy-tri for review.
cc @jwnimmer-tri @calderpg-tri @EricCousineau-TRI
Reviewable status: 0 of 1 files reviewed, all discussions resolved (waiting on @sammy-tri)
There was a problem hiding this comment.
Just a couple minor issues, also waiting a bit to give @jwnimmer-tri a change to weigh in on the telemetry message PR.
Reviewed 1 of 1 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @siyuanfeng-tri)
kuka-driver/kuka_driver.cc, line 98 at r1 (raw file):
namespace kuka_driver { class TimeSyncInterface {
I'm not sure the interface class is buying us much here. It doesn't even include all of the public functions the driver uses.
kuka-driver/kuka_driver.cc, line 154 at r1 (raw file):
} uint64_t num_measurement_;
This should probably be declared const?
There was a problem hiding this comment.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @siyuanfeng-tri)
a discussion (no related file):
I updated WORKSPACE to build against current drake/bazel. Once the drake PR merges, can you update WORKSPACE in this PR with the necessary drake sha and check that it builds locally?
|
sure. |
siyuanfeng-tri
force-pushed
the
dev/time_sync
branch
from
af38db6 to
334d8d9
Compare
There was a problem hiding this comment.
Reviewable status: 0 of 3 files reviewed, 3 unresolved discussions (waiting on @sammy-tri and @siyuanfeng-tri)
kuka-driver/kuka_driver.cc, line 98 at r1 (raw file):
Previously, sammy-tri (Sam Creasey) wrote…
I'm not sure the interface class is buying us much here. It doesn't even include all of the public functions the driver uses.
Done.
kuka-driver/kuka_driver.cc, line 154 at r1 (raw file):
Previously, sammy-tri (Sam Creasey) wrote…
This should probably be declared
const?
Done.
There was a problem hiding this comment.
Reviewed 3 of 3 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @siyuanfeng-tri)
There was a problem hiding this comment.
Reviewable status:
complete! all files reviewed, all discussions resolved
a discussion (no related file):
Previously, sammy-tri (Sam Creasey) wrote…
I updated
WORKSPACEto build against current drake/bazel. Once the drake PR merges, can you updateWORKSPACEin this PR with the necessary drake sha and check that it builds locally?
Done.
This change is