Feature/discovery #4
Conversation
reset to an empty discovery file on disabling discovery. This allows to enable discovery in all terminals without having run hector discovery initially.
checked at two places
| #!/bin/bash | ||
|
|
||
| # Create empty config on first usage | ||
| tmp_directory="/tmp/tuda_wss" |
There was a problem hiding this comment.
These files are sourced, hence, this variable will continue to exist after this script.
You should probably unset it after you are done using it.
Also I'm not sure if we should just overwrite the fastrtps config for everyone.
We should probably discuss this and if there is no better solution at least warn people if we are overwriting an existing FASTRTPS_DEFAULT_PROFILES_FILE variable with a different value.
There was a problem hiding this comment.
I can add the unset and probably also the warning.
For alternatives, I found only the option to set environment variables for the parameters, which could create conflicts with the FASTRTPS_DEFAULT_PROFILES_FILE.
It seems that ros by default only looks on the default profile or a locally provided config for an application.
https://docs.vulcanexus.org/en/jazzy/rst/tutorials/core/qos/initial_peers/initial_peers.html#xml-configuration-file-location
| def __call__(self, prefix, parsed_args, **kwargs): | ||
| complete_args = [var for var in load_robots().keys()] | ||
| # allowing a local server with ID 0 and default port | ||
| complete_args.append("local_server") |
There was a problem hiding this comment.
Why is this called local_server instead of localhost?
There was a problem hiding this comment.
The idea was to indicate that a local server still needs to be started. It can be changed to localhost.
|
|
||
|
|
||
| # dedicated place for temporary files | ||
| tmp_directory = "/tmp/tuda_wss" |
There was a problem hiding this comment.
Constants are usually SCREAMING_SNAKE_CASE.
Also this file was not black formatted.
There was a problem hiding this comment.
Maybe would make sense to declare this somewhere else and use it everywhere since at some point also other commands might use the tmp directory and currently this variable is just redefined everywhere.
| protocol.text = "SUPER_CLIENT" | ||
| tree.write(super_client_xml_path, encoding="utf-8", xml_declaration=True) | ||
|
|
||
| def _create_dds_xml(discovery_servers: list[DiscoveryServer]) -> ET.Element: |
There was a problem hiding this comment.
Currently this is only FastRTPS, can we abstract this more in some way so we can more easily add support for other middleware?
There was a problem hiding this comment.
From what I have read, it seems that the fastdds server is for FastRtps only. It seems no to be directly supported by other middleware. It could be possible, but it seems much more complicated.
There was a problem hiding this comment.
Yes, but other middlewares have analogues for the discovery server. E.g. zenoh has routers.
|
|
||
|
|
||
| class DiscoveryServer: | ||
| def __init__(self, address: str, port: int, guid_prefix: str): |
There was a problem hiding this comment.
Is this a guid, a guid prefix or a prefix guid?
| filtered_robots = [] | ||
| for robot_name, robot_data in robots.items(): | ||
| if robot_name == name: | ||
| filtered_robots.append(robot_data) | ||
| if len(filtered_robots) == 1: | ||
| discovery_servers.extend(filtered_robots[0].discovery_servers) | ||
| elif len(filtered_robots) == 0: | ||
| print_warn(f"Couldn't find correct entry for {name} in robot configs. Please check if your selected robot is available.") | ||
| else: | ||
| print_warn(f"Found multiple robot entries for {name} in robot configs. Your configs seem to be misformed.") |
There was a problem hiding this comment.
This is impossible. robots is a dict. It can't have multiple entries with the same key.
| print(super_client_xml_path) | ||
| print(discovery_xml_path) |
Feature to add a discovery command to the workspace scripts.
The discovery command reads a list of discovery servers from the robot configs and creates a discovery client XML.
The command can auto-complete and known robots, a local_server, all available robots, or disable the discovery server XML.
If future DDS settings are needed, they might have to be added to the same config.