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.
| @@ -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?
| @@ -111,6 +118,9 @@ def __init__(self): | |||
| # Setup the RTCM publisher | |||
| 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
| @@ -27,8 +27,8 @@ class NTRIPClient: | |||
|
|
|||
| # Public constants | |||
| 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
| @@ -195,7 +196,7 @@ def reconnect(self): | |||
| self._logerr('Reconnect to http://{}:{} failed. Retrying in {} seconds'.format(self._host, self._port, self.reconnect_attempt_wait_seconds)) | |||
| 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
This addresses SNIP sandboxing fix #55