Create v1.5.0 which fixes rtk2go rejection, retries reconnect, slows rate#71
Open
PaulBouchier wants to merge 2 commits into
Open
Create v1.5.0 which fixes rtk2go rejection, retries reconnect, slows rate#71PaulBouchier wants to merge 2 commits into
PaulBouchier wants to merge 2 commits into
Conversation
- Back out changes to ntrip_ros_base class, so serial device operation is unchanged - Fix error in retry with exponential delay logic - Add send_nmea boolean launch arg, controlling whether to send NMEA to caster - Exit with error if host is rtk2go and send_nmea is true, to avoid the user IPs and the ntrip_client getting banned - Make network connection retry behavior different than serial connection retry behavior (avoid changes to serial)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR is a new release of ntrip_client (v1.5.0), complete with updates to CHANGELOG.rst and README.md and package.xml, as well as the code files. It should be ready to merge to make a new release - no work required from the maintainer. It fixes issue #70 and other issues. This version has been tested on ROS Jazzy, but should work on all currently supported versions of ROS2.
Background: rtk2go.com effectively banned all previous versions of the LORD/Microstrain ntrip_client by blocking connections which report that the client is ntrip_client_ros. They did so on account of its bad behavior, where it sends nmea strings at a fixed rate of 10Hz. (1 Hz is recommended). Among other changes, this PR changes the reported client from "ntrip_client_ros" to "ros_ntrip_client", to bypass the rtk2go block, and makes the nmea rate configurable but defaulted to 1 Hz.
The biggest risk with this version is that it could break some system that depends on the nmea rate being 10Hz. The rate can be cranked up to 10 Hz, so such a system can overcome such a break by adding a launch file parameter that sets the nmea rate. I recommend defaulting the rate to 1 Hz as is done in this version - you just shouldn't need to send nmea messages at 10 Hz. If this is considered by the maintainer to be too big a risk, it would be possible to provide better backward compatibility by setting the rate to 1 Hz only if the caster target is rtk2go, at the cost of keeping the bad behavior of too-fast nmea messages for all other targets.
The change list is:
The persistent reconnect attempts use an exponential backoff delay, and handle initial connection failures as well as connection failures that occur during operation.
This version obsoletes (by introducing fixes) the following PRs: PR #65 PR #64, PR #69 which should all be closed as rejected because this version supercedes them.