open-rmf / rmf

Root repository for the RMF software
Apache License 2.0
225 stars 58 forks source link

Enum support for JSON schemas #108

Closed koonpeng closed 1 year ago

koonpeng commented 2 years ago

Feature request

Description

JSON schemas is often used to provide a wide range of utilities like docs, data validation and type checking for restful and other web-based applications, one solution to obtain json schemas for rmf data models and apis is to convert it from ros message. It works in most cases, but an important missing component is the support for enums.

Ros definitions does not support enums, instead, it uses constants to define fixed values. The problem is that there is no mapping between the constants and which field they are meant for, so there is no way to generate json schemas which contains enum values.

Implementation considerations

A simple solution is to introduce a style guide that enforces a certain naming pattern for constants in a way that can be mapping to enum values for a field. Extra tests can be run on CI to ensure that new PRs do not break the rules.

Alternatives

A more advanced approach is to define a custom definition format that is superset of ros definitions, a tool will then compile them to ros definitions. The json schema generators will use this custom definition instead of the generated ros definitions.

Another alternative is to do nothing, the need to convert ros definitions to json schema is because rmf does not currently support any web facing public apis. In the future, rmf is expected to have first class support for rest and websockets so there is no need for a web app to generate it's own schemas. However, the approach rmf uses to create it's json schema is still up for debate.

Additional information

mxgrey commented 2 years ago

With the changes that are forthcoming in https://github.com/open-rmf/rmf_task/pull/39 (and other PRs that are yet to be opened), we will be defining an RMF API that is based entirely on JSON schemas that are designed to be fit for use by web ecosystems, rather than converting from ROS messages that were meant for a ROS ecosystem.

We can keep this issue open in the meantime, but I think it'll be deprecated once we migrate to the new API.

Yadunund commented 1 year ago

Closing since we migrated to the new APIs quite some time ago.