Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[7138] Bugfix/discovery server localhost #971

Merged
merged 10 commits into from
Jan 30, 2020

Conversation

MiguelBarro
Copy link
Contributor

Applies the same treatment given to the locators received via DATA, to the locators pass into the RemoteServerAttributes.
Basically the locators become localhost if possible to speed things up.

@MiguelBarro MiguelBarro requested a review from EduPonz January 23, 2020 08:51
@MiguelBarro MiguelBarro self-assigned this Jan 23, 2020
@MiguelBarro MiguelBarro changed the title Bugfix/discovery server localhost [7138] Bugfix/discovery server localhost Jan 23, 2020
EduPonz
EduPonz previously approved these changes Jan 23, 2020
@richiware
Copy link
Member

Build status:

  • Linux Build Status
  • Mac Build Status
  • Windows Build Status

* if possible
* @param nf NetworkFactory used to make the translation
*/
void transform_server_remote_locators(NetworkFactory & nf);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
void transform_server_remote_locators(NetworkFactory & nf);
void transform_server_remote_locators(
const NetworkFactory & nf);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@@ -143,6 +145,21 @@ bool BuiltinProtocols::updateMetatrafficLocators(LocatorList_t& loclist)
return true;
}

void BuiltinProtocols::transform_server_remote_locators(NetworkFactory & nf)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
void BuiltinProtocols::transform_server_remote_locators(NetworkFactory & nf)
void BuiltinProtocols::transform_server_remote_locators(
const NetworkFactory & nf)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@richiware
Copy link
Member

Build status:

  • Linux Build Status
  • Mac Build Status
  • Windows Build Status

Copy link
Member

@MiguelCompany MiguelCompany left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some minor styling

include/fastrtps/rtps/builtin/BuiltinProtocols.h Outdated Show resolved Hide resolved
include/fastrtps/rtps/builtin/BuiltinProtocols.h Outdated Show resolved Hide resolved
src/cpp/rtps/builtin/BuiltinProtocols.cpp Outdated Show resolved Hide resolved
src/cpp/rtps/builtin/BuiltinProtocols.cpp Outdated Show resolved Hide resolved
@richiware
Copy link
Member

Build status:

  • Linux Build Status
  • Mac Build Status
  • Windows Build Status

@richiware
Copy link
Member

Build status:

  • Linux Build Status
  • Mac Build Status
  • Windows Build Status

@richiware
Copy link
Member

Build status:

  • Linux Build Status
  • Mac Build Status
  • Windows Build Status

MiguelCompany
MiguelCompany previously approved these changes Jan 29, 2020
@richiware
Copy link
Member

Build status:

  • Linux Build Status
  • Mac Build Status
  • Windows Build Status

@richiware
Copy link
Member

Build status:

  • Linux Build Status
  • Mac Build Status
  • Windows Build Status

@richiware
Copy link
Member

richiware commented Jan 30, 2020

Build status:

  • Linux Build Status (unrelated failure)
  • Mac Build Status
  • Windows Build Status (unrelated failure)

@MiguelCompany MiguelCompany merged commit f3f2c3d into 1.9.x Jan 30, 2020
@MiguelCompany MiguelCompany deleted the bugfix/discovery-server-localhost branch January 30, 2020 08:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants