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

[REP-2011] Evolving Message Types #358

Open
wants to merge 22 commits into
base: master
Choose a base branch
from

Conversation

wjwwood
Copy link
Contributor

@wjwwood wjwwood commented Jun 10, 2022

This REP (tentatively REP-2011) proposes patterns and approaches for evolving message, service, and action ROS interface types over time.

See the draft in the changes of this pr for information.

Thanks to @methylDragon, @youliangtan, @kylemarcey, and others for their help on this REP.


Status

Generally the REP is complete enough to invite technical feedback and discussions. Where things are poorly described, rationalized, or a particular alternative isn't listed, please point it out, but keep in mind that we may just haven't gotten to it yet.

  • REP writing by section:

    • Motivation: done
    • Specification: done, with TODOs
    • Rationale: partially done, with TODOs and a few places that need to be expanded
    • Backwards Compatibility: not started
    • Feature Progress: not started
  • Specification Reference Implementation:

    • Type Version Enforcement: not started
    • Type Description Distribution: not started
    • Run-Time Interface Reflection: not started
    • Tools for Evolving Types: not started

Known Issues

  • The REP needs to be updated to consider Services and Actions in several places
  • The parts about TypeDescription should be factored out into a separate section and referenced from the Type Version Enforcement, Type Description Distribution, and Run-Time Interface Reflection sections
  • The type version hash should probably be based on the TypeDescription, but the question about type name being part of the hash needs to be resolved first
  • The Run-Time Interface Reflection section should be expanded or split into the part about reflecting on serialized buffers using a TypeDescription and the part about creating "type support" structs for creating publishers and subscriptions from a TypeDescription
  • It is not yet clear how we will use the transfer functions for Services. It could be as a relay node like topics, or as plugins that are colocated with the Service client or server
  • There are some possibly upcoming changes to what is supported from .idl files, e.g. like enums, which may impact the TypeDescription type, and more generally we might want to consider how future changes to this type might be mitigated, e.g. by coming up with a way to handle .idl annotations in the type
  • We need a way to represent the "user-defined interface version" in either .idl or .msg files or both, or otherwise remove the idea from the REP, though it would be nice to preserve it
  • And many other smaller todo items left in the REP

wjwwood and others added 8 commits February 15, 2022 15:55
Signed-off-by: William Woodall <[email protected]>
Signed-off-by: William Woodall <[email protected]>
Signed-off-by: methylDragon <[email protected]>

Co-authored-by: William Woodall <[email protected]>
* Increase readability, fix typos, and reduce ambiguities

Signed-off-by: methylDragon <[email protected]>

* Add clarification for field semantics

Signed-off-by: methylDragon <[email protected]>

* Add first draft of inter-process type description distribution

Signed-off-by: methylDragon <[email protected]>

* Add additional notes for tick-tocking the metatype

Signed-off-by: methylDragon <[email protected]>

* Add rationale section for inter-process type description distribution

Signed-off-by: methylDragon <[email protected]>

* Add additional note about sending type description metadata

Signed-off-by: methylDragon <[email protected]>

* Add alternatives section for type description distribution

Signed-off-by: methylDragon <[email protected]>

* Fix broken TODO:: directives

Signed-off-by: methylDragon <[email protected]>

* Apply rewording suggestions for type description distribution

Signed-off-by: methylDragon <[email protected]>

Co-authored-by: William Woodall <[email protected]>

* Apply rewording suggestions from code review

Signed-off-by: methylDragon <[email protected]>

Co-authored-by: William Woodall <[email protected]>

Co-authored-by: William Woodall <[email protected]>

* remove trailing whitespace

Signed-off-by: William Woodall <[email protected]>

* fixups while walking through it with Brandon

Signed-off-by: William Woodall <[email protected]>

Co-authored-by: methylDragon <[email protected]>
Signed-off-by: William Woodall <[email protected]>
methylDragon and others added 9 commits July 7, 2022 17:48
* Add section on runtime type introspection

Signed-off-by: methylDragon <[email protected]>

* Apply REP rephrases

Signed-off-by: methylDragon <[email protected]>

Co-authored-by: William Woodall <[email protected]>

* Apply more REP rephrases

Signed-off-by: methylDragon <[email protected]>

Co-authored-by: William Woodall <[email protected]>
* Add nested type description section

Signed-off-by: methylDragon <[email protected]>

* Use Field type in message descriptions

Signed-off-by: methylDragon <[email protected]>

* Fix unmatched parenthesis

Signed-off-by: methylDragon <[email protected]>

* Implement nested introspection API

Signed-off-by: methylDragon <[email protected]>

* Rework nested introspection API

Signed-off-by: methylDragon <[email protected]>
Signed-off-by: William Woodall <[email protected]>
Signed-off-by: William Woodall <[email protected]>
Signed-off-by: William Woodall <[email protected]>
Signed-off-by: William Woodall <[email protected]>
@wjwwood wjwwood marked this pull request as ready for review August 22, 2022 05:32
@ros-discourse
Copy link

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/rfc-rep-2011-evolving-message-types/27006/1

@james-rms
Copy link

Hello! Glad this is moving along.

  1. Do you have any ideas or vision around how the Type Description would be serialized?
  2. Do you have any ideas about how ros2 bag play would provide this type description for topics recorded using older types?

@wjwwood
Copy link
Contributor Author

wjwwood commented Aug 24, 2022

Do you have any ideas or vision around how the Type Description would be serialized?

Sure, as described in the "Type Description Distribution" section, it will be a ROS message, called TypeDescription.msg (probably). So it will be serialized using the chosen rmw implementation. Inside the bag file it may be serialized as the rmw implementation would (e.g. CDR) or we could store it in a different format (e.g. json or yaml) if we wanted. We can also store the original description text and use that to recreate the TypeDescription instance.

Do you have any ideas about how ros2 bag play would provide this type description for topics recorded using older types?

Yeah, the idea of how it would work goes like this:

  • when doing ros2 bag record rosbag will save the TypeDescription instance and the original description text from the .msg/.idl file into the bag file by calling the ~/_get_type_description service on the node that is publishing the data
  • when doing ros2 bag play, the TypeDescription instance will be retrieved from the bag file and used to create a publisher
  • creating the publisher will give the node the type description at the version that was recorded into the bag originally

Anyone asking the rosbag node about that type through the ~/_get_type_description service will get the original description from the bag file as that's what was used to create the publisher.

For bag files that were created before this feature is implemented, we may be able to have a way to "augment" the bag file during playback, specifying a file which contains a copy of the type description text for the types at those versions in the bag file.
You would have to go back and get that information some how, but you could do that from the git history in most cases.

Copy link
Contributor

@vmayoral vmayoral left a comment

Choose a reason for hiding this comment

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

As a general comment, this is a great addition to ROS 2 and a fantastic REP proposal. Very well explained. I wish we could've had this long ago, as it would have saved much trouble in past projects.

I dropped some comments and questions below.

Abstract
========

This REP proposes patterns and approaches for evolving message, service, and action ROS interface types over time.
Copy link
Contributor

Choose a reason for hiding this comment

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

Take this as the input of a non-native speaker but I miss in here a bit of a short-ish "what for" in the abstract. Evolving message, service, and action ROS interface types over time is not self-explanatory. I know that the motivation section comes right afterwards but still, an abstract should land this IMHO. I read it a few times and still missed it. Maybe something along these lines:

Suggested change
This REP proposes patterns and approaches for evolving message, service, and action ROS interface types over time.
This REP proposes patterns and approaches for evolving message, service, and action ROS interface types over time. Such capability can be used for evolving ROS 2 itself over time while addressing interoperability issues due to mistmaching type definitions.

As a side observation, AFAIK, when compared to other REPs, the abstract in here is significantly larger.

The Motivation section is well reasoned.

rep-2011.rst Show resolved Hide resolved

sensor_msgs/msg/Image__RIHS1_XXXXXXXXXXXXXXXXXXXX

This has the benefit of "just working" for most middlewares which at least match based on the name of the type, and it is simple, requiring no further custom hooks into the middleware's discovery or matchmaking process.
Copy link
Contributor

Choose a reason for hiding this comment

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

Any consideration to be made on the max. lengths of the type names? Any restrictions to consider? Given the concatenated type version hash there might be little margin for using namespaces (which larger graphs tend to use).

Choose a reason for hiding this comment

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

That's a good point, I suppose we'd have to consider the DDSI-RTPS spec that limits topic lengths to 256 characters...

Though, looking at what such a character length even looks like:

sensor_msgs/msg/Image__RIHS1_XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX

I think there's plenty of space for multiple nested namespaces? I'd imagine the type version hash wouldn't need more than 32 characters to have a relatively low chance of collision, but that's just a guess off the top of my head.

In which case a topic name would have 224 characters, which is still a lot.

sensor_msgs/msg/Image__RIHS1_XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, since we're speaking about type names here (not topic names), the convention is to use <package name>/<type, msg/srv/action>/<name>, and so deeply nested namespaces isn't really as big of an issue as it is with things like topic names and node names.

In table 9.12 of the DDSI-RTPS 2.2 spec (https://www.omg.org/spec/DDSI-RTPS/2.2/PDF) it says the type name is limited to 256 characters as well. So the limit is the same, but again, I don't think we're in danger here of running into this limitation in practical situations. At least not very often.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One exception to this is the feature that allows you to configure topic endpoint's QoS using parameters: http://design.ros2.org/articles/qos_configurability.html

In this case the topic name is part of the parameter name, but unlike topic/service/node names, we don't have a limit for the length of parameter names at the moment.

rep-2011.rst Outdated
It should be along side the ``topic_type`` string in that struct, but the ``topic_type`` field should not include the concatenated type version hash, even if the recommended approach is used.
Instead the type name and version hash should be separated and placed in the fields separately.

This information should be transmitted as part of the discovery process.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if I understand properly the meaning of the "discovery process" in here.

To my understanding, generally, for most DDS implementations, discovery traffic is sent prior/outside of the security plugins domain (i.e. this information can be used for reconnaissance). If this information is sent also as part of the DDS discovery process, won't it mean that we'd be conveying it in an uncrypted manner (regardless of whether security is enabled or not in the graph)?

Also, (a final answer would of course depend on the implementation but) can you briefly comment on what's the expectation regarding additional traffic generated due to sending this information as part of discovery? Will it be just the type version hash for each ROS interface?

Choose a reason for hiding this comment

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

Well, topic names are already sent as part of the rmw_topic_endpoint_info_t struct. Adding a hash in my opinion is not much of a security risk, since the hash would be degenerate, and by design, meant to not be reversible.

Furthermore, depending on how the hash is generated (e.g. factoring in the field names in the type description), it can be made more robust to rainbow table attacks (definitely more than something like, say, brute forcing of MD5 hashes for passwords.)


Regarding additional traffic, I'd think it'd just be the type version hash. The rest of the stuff (e.g. type description distribution) is planned to be sent after discovery, after the participants have determined that they're mismatched via the hash on discovery.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@methylDragon is right, it should just be the version hash, which would be in addition to the type name which already being sent via discovery. So the impact to information leak and extra bandwidth used should be minimal. The hash is going to be a sha-1 of the type description struct (most likely) so that is reversible, but not easily and not for much gain I think.

The actual full type descriptions (and other meta data) would be sent via the ~/_get_type_description service which will have to be considered in security set ups just all the other built-in node topics/services must.

rep-2011.rst Outdated
Type Description Contents and Format
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

The response sent by the ROS Service server will contain a combination of the original ``idl`` or ``msg`` file's content, as well as any necessary information to serialize and deserialize the raw message buffers sent on the topic.

Choose a reason for hiding this comment

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

It's probably an important detail that the full idl or msg definition of all field types needs to be provided along with the toplevel definition, otherwise there may be an unresolved incompatibility between types at a field level. It's worth noting that ROS1 tries to solve this by concatenating all of its msg definitions using gendeps --cat and sending this in the connection header for new connections.

Choose a reason for hiding this comment

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

Yup! You're right! They're meant to be sent along the main type description under the referenced_type_descriptions field.

At least, for any non-primitive types.

Do you see value in concatenating the referenced file contents as well (including the comments)? Or would a parsed definition suffice?

Choose a reason for hiding this comment

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

Do you see value in concatenating the referenced file contents as well (including the comments)? Or would a parsed definition suffice?

It depends on how frozen-in-time the parsing procedure is. If the parsed definition evolves over time, I'd keep the original file content around, because then if there's a Type Description schema mismatch, a node can fall back to rebuilding the type description from the original file content. Stripping comments seems safe enough, since at least then the content is still valid IDL or MSG.

Copy link
Contributor

@amacneil amacneil Sep 13, 2022

Choose a reason for hiding this comment

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

@wjwwood regarding storing a type definition source (msg or idl) as a string, I wanted to call your attention to work that has been happening over in mcap.

The specific problem I am referencing is how to store both the message definition and all direct and transitive dependencies in the proposed type_description_raw field. ROS 1 solves this using the gendeps --cat format (mentioned by @james-rms above), which consists of msg files delimited by 80 = characters plus a message name line.

For storing concatenated source message definitions in MCAP, we opted to follow the ROS 1 convention for msg files, which we use for both ROS 1 and ROS 2 data. For IDL source message definitions, we modified the concatenation slightly so that all types including the top level type are explicitly named.

You can see our specification for this here, and discussion on the spec PR at foxglove/mcap#553. This spec was then implemented in ros-tooling/rosbag2_storage_mcap#53.

I think it would be best if we can use this same concatenation standard for REP 2011 types.

Copy link
Contributor

Choose a reason for hiding this comment

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

It was pointed out to me that you could avoid concatenating message definition strings entirely by providing an array of types (the top level type and any dependent types, each with a name, format, and string body).

Signed-off-by: William Woodall <[email protected]>
Signed-off-by: William Woodall <[email protected]>
@ros-discourse
Copy link

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-2-tsc-meeting-minutes-2022-9-15/27394/1

@ros-discourse
Copy link

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-2-tsc-meeting-minutes-2022-10-13/28213/1


.. TODO::

discuss the implications for large messages and the possibility of having the transfer functions be colocated with either the publisher or subscription more directly than with component nodes and remapping.
Copy link
Contributor

Choose a reason for hiding this comment

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

probably this depends on version mismatch situation. if each subscription does have different message version, that would be efficient that transfer functions should be processed on subscription, since it will delay the publisher to send the message out to the network.
on the other hand, and i think this is likely that maybe 1 or 2 version mismatch and multiple subscriptions in the topology. in that case, transfer functions should be processed in publisher side to avoid to call transfer function on each subscription.

besides, this would be nice to configured? because it depends on the platform resource, no matter what mismatch situation it is, platform resource could be the problem for this extra transfer function, thinking about far edge devices.


These features are described in the following sections.

Type Version Enforcement
Copy link
Contributor

Choose a reason for hiding this comment

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

It is more like detection as feature? user must be able to know if the type version mismatches or not, then it is up to user application to enforce and transfer the version?

Copy link
Contributor

@emersonknapp emersonknapp left a comment

Choose a reason for hiding this comment

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

Some "my 2 cents" mostly on TODO items, couple questions, clarifications

rep-2011.rst Outdated

.. TODO::

Related TODO, should we just use the TypeDescription described in a later section?
Copy link
Contributor

Choose a reason for hiding this comment

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

I think yes. Has there been further discussion on this point, as to whether the has should be based on anything other than the TypeDescription? It seems like TypeDescription peels away the unrelated info (comments, default values, etc.) leaving only the relevant information to hash.

rep-2011.rst Outdated

.. TODO::

Remove the idea of the user-defined interface version or define how it can be supplied by the user in one or more of the idl file kinds.
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps just allow users to version via a recommended strategy of having a "versionN: true" field that is replaced for new versions? First thought was have it have a default value, or define a const, but I think based on current description of hashing logic, those two particular things should not change the hash. Inclined to agree that we don't need a special mechanism in this REP for "manual hash break".

rep-2011.rst Outdated

.. code::

sensor_msgs/msg/Image__RIHS1_XXXXXXXXXXXXXXXXXXXX
Copy link
Contributor

Choose a reason for hiding this comment

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

Small concern here, maybe just due to my bad memory - do we require all RMW impls to use the provided QoS profiles? If so, and we are planning to add USER_DATA to provide type hash, then do we need this backup strategy of concatenated type name?

If USER_DATA not universally viable - if one does use this strategy, is the plaintext typename totally redundant (because type name is in the TypeDescription object that produces the hash, and so prevents hash collision for types even of the same layout with different names)? If redundant, would it be better to just advertise type RIHS_1_xxx? Maybe not, since hash is not reversible, and the prefix allows for checking for transfer functions. I may have not been thinking of the whole scope of the REP.

Choose a reason for hiding this comment

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

Not sure about QoS profiles.

But for advertised type names.. Putting the type name as a prefix in the advertised type will allow us to bypass querying for the TypeDescription object in the case where we just want the name of the type the participant is trying to use (e.g. in a tool to see what participants are trying to communicate using the same type (the prefix), but can't because of the layout (different hashes))

rep-2011.rst Outdated

.. TODO::

It's not clear how we will do this just now, since existing "QoS events" lack a way to communicate this information, I (wjwwood) think.
Copy link
Contributor

Choose a reason for hiding this comment

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

QoS events lack a way to communicate which information? The specific endpoint? The current events, provided in callbacks, do carry event-specific struct payloads that are filled out by the RMW implementation. The current ones are all simple number types at the moment, but that's not a hard requirement. I don't see a reason why rmw_topic_endpoint_info_t couldn't be included in the callback payload even, or just the GID or something from it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Noting that this callback type is now implemented, so this TODO (and my prior comment) are resolved

rep-2011.rst Outdated

.. TODO::

How can we detect when a remote node is not offering this service?
Copy link
Contributor

Choose a reason for hiding this comment

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

Just agreeing that yes this is challenging. rosbag2 is going to be a first adopter of these services, and have a strong preference for finding type descriptions. How will it know when to stop checking? Like you've pointed out, hard to tell whether not ready yet, or never will be.

rep-2011.rst Outdated

.. TODO::

Should each endpoint be asked about their type/version pair or should we assume that the type/version pair guarantees a unique type description and therefore reuse past queries?
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems to me best to make a strong statement that type/version pair is a unique type description so a user can expect that anybody with that name and hash has the same type. From bag recording perspective, with potentially a lot of topics, it'd be nice to only call out once per unique type, and I'd probably just base it on hash lookup in internal definition cache, whether to call or not

Choose a reason for hiding this comment

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

+1

rep-2011.rst Outdated
.. TODO::

What happens if the message consumer doesn't have access to the serialization library stated in the meta-type?
(wjwwood) It depends on what you're doing with the response. If you are going to subscribe to a remote endpoint, then I think we just refuse to create the subscription, as communication will not work, and there's a basic assumption that if you're going to communicate with an endpoint then you are using the same or compatible rmw implementations, which includes the serialization technology. The purpose of this section and ROS Service is to provide the information, not to ensure communication can happen.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's fair to say that cross-protocol communications is out of scope for this REP, even if we are providing information that could unlock it down the line.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed. That said, I think this part of the discussion should be preserved, possibly by moving this paragraph out of the TODO section and doing some editing.

rep-2011.rst Outdated

.. TODO::

(wjwwood) I propose we use key-value string pairs for the extra information, but I am hoping for discussion on this point.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe I missed it, but what extra_information are we anticipating here? Not sure what the value of debating representation is, without a sense of what it's for.

Choose a reason for hiding this comment

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

I think this is more used as a final catch-all to mitigate the case where a future feature/bug getting uncovered forces us to release a new version of the type description message

rep-2011.rst Outdated

.. TODO::

(wjwwood) A thought just occurred to me, which is that we could maybe have "lazy" Service Servers, which watch for any Service Clients to come up on their Service name and when they see it, they could start a Service Server.
Copy link
Contributor

Choose a reason for hiding this comment

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

What overhead is presented by a ServiceServer? Unlike a Publisher, it doesn't do work unless called, so I'm not seeing a comparison to the lazy publisher. Is it discovery overhead that's the main concern, or is there also a non-negligible memory/cpu contribution to creating the server? How does that overhead compare to the monitoring required to see whether there are any prospective Clients?


This section needs to be updated to be inclusive to at least Services, and then we can also mention Actions, though they are a combination of Topics and Services and so don't need special support probably.

Plugin Interface
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: should this be titled "Serialization Library Plugin Interface"?

Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

I realize that I never left my actual review. I have two comments inline so far, one of which is already addresses in a PR to this PR.

rep-2011.rst Outdated
It's essentially the same thing, but it does include the type name.
I (wjwwood) am leaning in this direction.

The resulting data structure is hashed using a standard SHA-1 method, resulting in a standard 160-bit (20-byte) hash value which is also generally known as a "message digest".
Copy link
Contributor

Choose a reason for hiding this comment

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

At the risk of opening a can of worms, I think we should consider whether we really want to use SHA-1 here. While I understand this isn't meant to be used cryptographically, all of the signs seem to say that we should be migrating away from SHA-1 now. So unless we absolutely have to use SHA-1 for some kind of compatibility, I feel like we should choose a different hashing algorithm (maybe SHA-256).

rep-2011.rst Outdated
.. TODO::

What happens if the message consumer doesn't have access to the serialization library stated in the meta-type?
(wjwwood) It depends on what you're doing with the response. If you are going to subscribe to a remote endpoint, then I think we just refuse to create the subscription, as communication will not work, and there's a basic assumption that if you're going to communicate with an endpoint then you are using the same or compatible rmw implementations, which includes the serialization technology. The purpose of this section and ROS Service is to provide the information, not to ensure communication can happen.
Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed. That said, I think this part of the discussion should be preserved, possibly by moving this paragraph out of the TODO section and doing some editing.

@ros-discourse
Copy link

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-2-tsc-meeting-minutes-2023-03-16/30432/1

@ros-discourse
Copy link

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-2-tsc-meeting-minutes-4-20-2023/31087/1

Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

We went over this, and we are considering splitting this up into a few REPs:

  1. Type Description, Type Hash, and Type Description Service
  2. Run-Time Interface Reflection - Dynamic Types, Dynamic Data, Dynamic .msg, and typesupport
  3. Evolving message types (REP-2011)

* Full pass removing type descriptions/hashes portion for new rep

Signed-off-by: Emerson Knapp <[email protected]>

* Starting a second pass cleaning up the edits

Signed-off-by: Emerson Knapp <[email protected]>

* Tiny changes

Signed-off-by: Emerson Knapp <[email protected]>

* More tinies

Signed-off-by: Emerson Knapp <[email protected]>

---------

Signed-off-by: Emerson Knapp <[email protected]>
@wjwwood
Copy link
Contributor Author

wjwwood commented Sep 12, 2023

As of wjwwood#9 being merged, a big chunk of this REP was moved officially into REP-2016 (#381).

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.

10 participants