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

Application-specific types are back #73

Merged
merged 18 commits into from
Dec 18, 2019

Conversation

pavel-kirienko
Copy link
Member

New stuff:

  • Minor refactoring of unstable data types in uavcan.time to facilitate their reuse in future application-specific namespaces. No semantic changes.
  • Added new SI types for force, torque, and frequency. Torque and frequency are used in the new namespace, force is added for completeness.
  • Added an application-specific namespace robotics (read below) intended for software-defined vehicular systems at large. The new namespace contains data types for the following applications so far:
    • ESC, primarily intended for multirotors and other types of electric aircraft.
    • Smart battery (BMS).
  • Updated the documentation in the README.

This pull request should resolve the recurring argument about the removal of application-specific data types from the standard set in UAVCAN v1. A few months ago, it prompted Scott to write a FAQ entry specifically about standard data types; also, our presentation at the PX4 dev summit addressed the problem with the help of the following infographic:

typecat

Originally, the intent was to arrange regulated application-specific data types (such as BMS messages) in several categories, named profiles, and dedicate a separate namespace to each profile:

profiles

Upon a closer assessment, I concluded that a strict separation may be undesirable because the protocol encourages similar approaches to common problems that may be encountered in different application domains. If the original plan was followed, it would likely lead to a noticeable overlap between the functionalities offered by different profiles.

In this PR, I am proposing a modified approach that is closer to the model of a generic robotics framework, where all profile-agnostic functionality is extracted into a generic namespace under a sufficiently generic name. I found robotics to suit the purpose, but new suggestions are welcome. This change does not preclude the addition of profile namespaces later, shall the need arise.

You will notice that the definitions lack fixed port-identifiers. This is by design: excessive reliance on those was one of the major problems in v0. The related issues are briefly explained in the recap of the Stockholm Summit. For the few (very few) types where a fixed port-ID will be found to be desirable one can always be added later, since it does not affect backward compatibility.

Right now, the main objective is to establish the general architecture. Once it is accepted, I am planning to extend the namespace with new commonly needed types such as GNSS positioning messages and related generic geometric types.

I invite the following people to look at the definitions and the namespace design to ensure their adequacy:

@thirtytwobits
Copy link
Member

I think we can discuss a generic application profile but I don't think this should be included in the standard. For example, if a system integrator defines their own actuator messages the inclusion of a generic actuator in the set of regulated types will cause confusion. I would rather we move your proposed robotics namespace to a repo which manages public, regulated, but generic types (public_regulated_generic_data_types perhaps?).

@pavel-kirienko
Copy link
Member Author

I would rather we move your proposed robotics namespace to a repo which manages public, regulated, but generic types (public_regulated_generic_data_types perhaps?).

Indeed, I am aware of the risk of confusion. Here, I attempted to eliminate it by explicitly differentiating between the standard namespace (uavcan) and everything else, as you can see in the diffs for README.md. I am inclined to avoid splitting our definitions across multiple sources (I might have been infected by your monorepo ideas) because the increase in the costs of maintenance is noticeable, whereas the only meaningful difference perceptible to the user is the change of the name from public_regulated_data_types to public_regulated_generic_data_types. Maybe we could save ourselves the effort and take a shortcut by avoiding the split and substitute that with better docs explaining what it is we are really offering here?

Or the namespace could be renamed into something that is less generic-looking than robotics?

We could also make it purely vendor-specific by renaming it into zubax but I am not sure yet about the public-relations and other non-technical implications of this change.

@pavel-kirienko
Copy link
Member Author

We just had a brief call with @thirtytwobits about this, and we concluded that currently, we are unlikely to be able to define a sufficiently generic and application-agnostic data type library because the UAVCAN development team does not have the required expertise. The updated plan is to move the proposed generic definitions to a new namespace zubax and use it as a stabilization playground. In the future, we may revisit the idea of an application-agnostic profile if there is sufficient interest and the adopters are willing to supply the necessary expertise.

@AlexKlimaj
Copy link

For the battery message. I think the only thing I would add to what you've got is the time remaining message that is in the mavlink smart battery status message.

@pavel-kirienko
Copy link
Member Author

For the battery message. I think the only thing I would add to what you've got is the time remaining message that is in the mavlink smart battery status message.

@AlexKlimaj Are you saying that a BMS may be able to calculate a substantially better estimate than a subscriber to battery status messages? The proposed interface is built around an estimate of the amount of energy that can be reclaimed from the battery; knowing the available energy and the current power consumption, the time-to-empty can be obtained by a trivial division of one to another.


The change from robotics to zubax is currently staged in a separate branch; will merge it here soon.

robotics/actuator/esc_group/TorqueSetpoint.0.1.uavcan Outdated Show resolved Hide resolved
robotics/sensor/bms/BatteryPackStatus.0.1.uavcan Outdated Show resolved Hide resolved
robotics/sensor/bms/BatteryPackStatus.0.1.uavcan Outdated Show resolved Hide resolved
The primary motivation for this is to illustrate to adopters that finer segregation may allow one to build more generic, context-agnostic interfaces.
@pavel-kirienko
Copy link
Member Author

Please review.

pavel-kirienko added a commit to OpenCyphal/nunavut that referenced this pull request Oct 30, 2019
This is needed to ensure that the latest patches are pulled in. They are needed for compatibility with OpenCyphal/public_regulated_data_types#73
thirtytwobits pushed a commit to OpenCyphal/nunavut that referenced this pull request Oct 30, 2019
This is needed to ensure that the latest patches are pulled in. They are needed for compatibility with OpenCyphal/public_regulated_data_types#73
README.md Outdated

Every vendor must have a dedicated root namespace.
The root namespace should be named after the vendor's legal entity name.
Non-standard regulated data types are contained in the root namespace `com`.
Copy link
Member

Choose a reason for hiding this comment

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

can this be a suggestion? Why com? Why not edu, io, ca, etc? Should we just allow any TLD?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it is critical that we enclose all vendor-specific namespaces in a top-level namespace because:

Having a vendor namespace named after said vendor in the public regulated repo would make it impossible for the same vendor to have an unregulated namespace under the same name. This is because having an identically named namespace with different contents is currently explicitly prohibited by the spec (OpenCyphal/pycyphal#68 (comment)).

The name of the top-level namespace does not matter much, but it's best to keep it short because we only have 50 characters for the full data type name. Using TLDs is not really critical, but I think they suit the purpose well because they are short, familiar, and their meaning is intuitively clear. I am not sure whether we should allow more than one TLD; maybe we should at least discourage that for reasons of consistency. We can always lift the restriction to use only com in the future. Do you think we should lift it now?

The domain name analogy should also make it intuitively clear that the public regulated repository is a common, globally shared resource, which should be well-structured and maintained carefully. Vendors are free to use other, less formal naming patters in their non-regulated or private namespaces (much like it's possible to use any TLD in a local DNS server).

Under the above model, the fact that uavcan is a separate namespace, not under a TLD, is desirable because it highlights its special status.

Copy link
Member

@thirtytwobits thirtytwobits Oct 31, 2019

Choose a reason for hiding this comment

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

Using TLDs is a good way to deconflict but we should support some range of them. I'd prefer one of two conventions be established:

  1. Support a small set of the most common TLDs that are 2-3 chars in length. For example the original 7 plus io : [com, org, net, int, edu, gov, mil, io]

  2. Support all IANA TLDs. This can be supported in parsers by downloading that list from http://data.iana.org/TLD/tlds-alpha-by-domain.txt

Copy link
Member Author

Choose a reason for hiding this comment

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

Emergent complexity is looming on the horizon.

The spec says that if you defined a top-level namespace in one place, then that's your entire namespace and you will have to keep it there as an atomic entity. Want to define a sub-namespace somewhere separate? Nope, you can't.

The reason is that having access to the entire namespace, a DSDL front-end (currently it's only PyDSDL) can enforce versioning rules and warn early about name conflicts. The point of aggregation could be chosen arbitrarily; it's a root namespace because it maps well onto the general protocol architecture, but it could be higher (multiple root namespaces?) or lower (sub-root namespaces?).

We may remove this restriction later, but as long as it is in place, we have to keep in mind that by allowing the use of a root namespace for the public regulated namespace library (this repo), we thereby withdraw it from the set of permissible names that vendors could use for themselves. This is why allowing any TLD from the most recent set standardized by IANA is a bad idea -- what if there is a weirdly named vendor like "MOSCOW" or idk "GOOGLE"?

Imagine now that we're living in the far future where the civilization has advanced to the point where one doesn't need to keep their root namespaces atomic to guarantee compatibility. Anybody is free to name their vendor-specific namespace after an Internet domain name, like com.nozama. Do we still want to provide a clear separation between public regulated types and non-regulated types? Do we still want to appease hipsters registering their websites under .io, or is that carefully crafted special case no longer relevant?

Under a working hypothesis that the optimal answers are "yes" and "no", we arrive at the conclusion that TLD-based regulated root namespaces may not age well. Further, as long as Specification requires atomic root namespaces, it should also prohibit usage of TLDs for unregulated root namespace names, and DSDL front-ends should refuse to process them. Seriously, there are v0 vendors out there that are already practicing this, not knowing that it is a recipe for disaster.

Should we search for a new name for the outer namespace of vendor-specific public regulated types instead of com?

Should we discuss the atomic namespace requirement in-depth?

Copy link
Member Author

Choose a reason for hiding this comment

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

We could consider replacing com with universe.

Copy link
Member

Choose a reason for hiding this comment

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

public?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a keyword in many languages and data types can be public-unregulated. Maybe just regulated? It is unambiguous because data types cannot be private and regulated:

image

Copy link
Member Author

Choose a reason for hiding this comment

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

Or reg because we care about length.

Copy link
Member

Choose a reason for hiding this comment

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

okay. reg but what about name-spacing here. With the core types we have everything under uavcan which is unlikely to result in collisions when generated code in combined with an existing codebase. All of the proposed top-level namespaces are generic enough that we may see collisions. Should we not have something with uavcan in it some how?

ucreg
uavcan-reg
uavcan.reg

?

Copy link
Member Author

Choose a reason for hiding this comment

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

I understand the name conflict argument and I thought about it too. Maybe it's best to solve this not at the infrastructure level but at the implementation level, allowing the user to specify what package or library should DSDL-generated code be wrapped in?

A quick look around reveals that reg conflicts with existing (albeit not very useful) things out there:

But regulated seems okay:

I agree though that it's weird to use such a generic adjective to name a software package. It makes sense for DSDL, but one scrolling through a codebase that relies on DSDL might be perplexed by things like #include <regulated/zubax/actuator/esc_group/Status_0_1.hpp>. Despite this, I don't have better ideas at the moment.

dimracer
dimracer previously approved these changes Nov 1, 2019
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
uavcan/time/TAIInfo.0.1.uavcan Show resolved Hide resolved
uavcan/time/TAIInfo.0.1.uavcan Show resolved Hide resolved
@pavel-kirienko
Copy link
Member Author

Okay. I will postpone merging this until we have discussed the regulated namespace management policy with @thirtytwobits and @LorenzMeier.

@pavel-kirienko pavel-kirienko merged commit 3ab1d38 into master Dec 18, 2019
@pavel-kirienko pavel-kirienko deleted the first-application-specific-namespace branch December 18, 2019 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants