[WIP] Synchronize time with Jetson using UDP #63
Conversation
snowsignal
requested review from
andrewda,
will-hou,
gnawinganimal and
evanmyersflow
|
Hi @JacksonCoder, thanks for opening this pull request! Looks like this is an addition or fix for the robot. To make sure this change goes added in smoothly, make sure the following chores are complete (please check-off items as they are completed):
Maintainers – please review this PR. A review from the @SouthEugeneRoboticsTeam/reviewers team is required before the PR can be merged. Finally, use Thanks! |
| } | ||
|
|
||
| // Extract seconds from an epoch number | ||
| private fun epoch_secs(epoch: Long): Long { |
There was a problem hiding this comment.
Everything should be camelCase
| } | ||
|
|
||
| // Extract milliseconds from the epoch number | ||
| private fun epoch_millis(epoch: Long): Long { |
| // UDP port | ||
| private const val PORT: Int = 2521 | ||
| // Change this to the Jetson's IP before deploying | ||
| private const val JETSON_IP = "127.0.0.1" |
There was a problem hiding this comment.
These should all be constants set in the constants.kt file.
There was a problem hiding this comment.
Except WAIT_PERIOD, which should stay here.
|
|
||
| object TimeSync : Thread() { | ||
| // Time (in secs) to wait between syncs | ||
| private const val WAIT_PERIOD: Long = 5 |
There was a problem hiding this comment.
Just do this in milliseconds, will make everything easier.
There was a problem hiding this comment.
Also no need to specify the type.
There was a problem hiding this comment.
Actually, we do need to specify this as Long, otherwise it gets inferred as an Int.
| // Time (in secs) to wait between syncs | ||
| private const val WAIT_PERIOD: Long = 5 | ||
| // UDP port | ||
| private const val PORT: Int = 2521 |
andrewda
changed the title
|
@andrewda, this can already broadcast as well. If we set |
|
I think we'll be fine with |
| @@ -42,6 +42,10 @@ const val MAX_VELOCITY = 0.6 | |||
| const val MAX_ACCELERATION = 0.1 | |||
| const val MAX_JERK = 60.0 | |||
|
|
|||
| // TimeSync | |||
| const val JETSON_PORT = 2521 // UDP Port for Jetson | |||
There was a problem hiding this comment.
This has gotta be a value between 5800 and 5810, but we're already using 5800 for Jetson -› Robot comms.
There was a problem hiding this comment.
Alright, let's use 5801, then.
|
I've also updated time-sync-server to be compatible with these changes. |
|
Should I write tests? |
|
@JacksonCoder Eh, not necessary for this. And besides we don't really have any tests on this repo. Next year, though... Oh man we're gonna test everything ;). |
This is still a work in progress, but I'm making a PR now to get some early feedback on this. This fixes issue #61.