-
Notifications
You must be signed in to change notification settings - Fork 90
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
adding tcp connection support #17
Conversation
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.
Hi Josh, thanks for your pull request.
Please have a look at my comments and let me know what you think about them.
Regards,
Marco.
base_station_mode: false | ||
debug_mode: false # if true, output every topic read by this driver | ||
|
||
# debug_mode: if true, output every topic read by this driver | ||
# whenever possible, use network broadcast, not global ip broadcast |
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 " whenever possible, use ..." geos before to parameter "broadcast_addr" and not before param "debug_mode".
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.
good catch. will update.
@@ -1,7 +1,7 @@ | |||
#!/bin/bash | |||
|
|||
GIT_REPO_LIBSBP=https://github.com/swift-nav/libsbp.git | |||
REPO_TAG=v2.2.15 #version you want to checkout before installing | |||
REPO_TAG=v2.3.11 #version you want to checkout before installing |
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.
Why do you want to change this in master? Master has been tested with SBP 2.2.15, as written here https://github.com/ethz-asl/ethz_piksi_ros/tree/master/piksi_multi_rtk_ros#firmware-and-sbp-lib-version
I'm working on another pull request to switch to 2.3.10: #16
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 had been working against 2.3.11 (which was the newest tag of SBP at the time) but 2.3.10 would be fine as well. I will adjust the PR. Is your PR scheduled to be merged soon? v1.4.10 of the firmware uses SBP version 2.3.1 - so I'd like to get support for the current firmware into the driver ASAP.
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.
We just deiced to stick to the indication here: Piksi Multi and Duro SBP Specifications.
I'd like to test my PR today and merge it asap ;)
@@ -45,7 +47,7 @@ | |||
|
|||
|
|||
class PiksiMulti: | |||
LIB_SBP_VERSION_MULTI = '2.2.15' # SBP version used for Piksi Multi. | |||
LIB_SBP_VERSION_MULTI = '2.3.11' # SBP version used for Piksi Multi. |
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 here, please leave 2.2.15
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 as previous.
serial_port: '/dev/ttyUSB0' | ||
|
||
# baud_rate: defines the rate for the serial connection. This driver uses | ||
# 234000 as the default. Swift Navigation receivers ship with a |
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.
Typo? Correct value is 230400
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.
good catch - will fix.
# baud_rate: defines the rate for the serial connection. This driver uses | ||
# 234000 as the default. Swift Navigation receivers ship with a | ||
# default baud rate of 115200. | ||
baud_rate: 234000 |
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.
Typo? Correct value is 230400
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.
good catch - will fix.
raise | ||
else: | ||
serial_port = rospy.get_param('~serial_port', '/dev/ttyUSB0') | ||
baud_rate = rospy.get_param('~baud_rate', 23400) |
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.
Typo? Correct value is 230400
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.
good catch - will fix.
Adding support for connection to receiver over TCP.
Adjusting print statements to be more generic, referencing "Swift receiver" rather than "Piksi" since the driver also supports the Duro receiver.