Feature/ros2 ntrip rate control#56
Conversation
Allows compliance with NTRIP servers with limits on how often communication is allowed. New parameter ntrip_server_hz sets the rate at which this client requests rtcm data from the server. defaults to the previous hard-coded 10hz, but should be set to 1hz for rtk2go.com NMEA sentences are cached and the latest one sent at the same time the rtcm data is requested.
robbiefish
left a comment
robbiefish
left a comment
There was a problem hiding this comment.
Sorry this took me so long to get to. This looks like a very useful feature to have
| @@ -15,8 +15,9 @@ def generate_launch_description(): | |||
| DeclareLaunchArgument('port', default_value='2101'), | |||
| DeclareLaunchArgument('mountpoint', default_value='VTRI_RTCM3'), | |||
There was a problem hiding this comment.
I like that you added an example of overriding in the README with rtk2go information. I think that would be the right thing to have in here as well. Can you update host, mountpoint, username, and password?
| self._rtcm_pub = self.create_publisher(self._rtcm_message_type, 'rtcm', 10) | ||
|
|
||
| # Setup a server frequency confirmation publisher | ||
| self._rate_confirm_pub = self.create_publisher(String, 'ntrip_server_hz', 10) |
There was a problem hiding this comment.
I think this publisher should be removed.
| # Publish a confirmation message to indicate the send_rtcm_and_nmea call | ||
| confirmation_msg = String() | ||
| confirmation_msg.data = "RTCM request and NMEA receive set at rate: {} Hz".format(1.0 / self.rtcm_request_rate) | ||
| self._rate_confirm_pub.publish(confirmation_msg) |
There was a problem hiding this comment.
Same as above. I would be okay having this as a debug log, but would rather not have an extra string publisher
| DEFAULT_RECONNECT_ATTEMPT_MAX = 10 | ||
| DEFAULT_RECONNECT_ATEMPT_WAIT_SECONDS = 5 | ||
| DEFAULT_RTCM_TIMEOUT_SECONDS = 4 | ||
| DEFAULT_RECONNECT_ATEMPT_WAIT_SECONDS = 10 #was 5 - changed for rtk2go reqs |
There was a problem hiding this comment.
These new defaults seem reasonable. The comment can be removed
| time.sleep(self.reconnect_attempt_wait_seconds) | ||
| elif self._reconnect_attempt_count >= self.reconnect_attempt_max: | ||
| self._reconnect_attempt_count = 0 | ||
| # self._reconnect_attempt_count = 0 #this hides the retry count from the excepton? |
There was a problem hiding this comment.
Yup this was a mistake. Just put this after the log, and that should be fine.
Realistically, it is probably fine to just not set it, but the ntrip_client.py is meant to be agnostic of how ROS runs it, so it is possible that someone could catch that exception and want the value to be updated properly
2 participants
This addresses SNIP sandboxing fix #55