-
Notifications
You must be signed in to change notification settings - Fork 6
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
[openapi] move or add fields from injection.yaml to commons.yaml #25
Conversation
84c77ab
to
414b836
Compare
rid/v1/commons.yaml
Outdated
minimum: 0 | ||
exclusiveMinimum: false | ||
type: number | ||
UAType: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In injection.yaml
this is known as RIDAircraftType
. Reason is that RIDAircraftType
is taken from v19 and UAType
is taken from v22a. The only practical difference seems that in v22a the value VTOL
is not present anymore.
However now for the testing interfaces we want a single interface and not two different ones.
Here I'd be tempted to:
- remove
RIDAircraftType
frominjection.yaml
- reference the common
UAType
here - add here the
VTOL
value with a note
That should result in a compatible API. WDYT? Maybe another subject for the weekly today.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interestingly, while RIDAircraftType
is defined in injection.yaml
, it is not used anywhere in the actual API definition (meaning we can change it at will without breaking anything)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed at the weekly meeting, will add the VTOL enum to UAType, and only keep UAType for the injection.
It will be the USS's job to map this to the v19 version of things if they are implementing that version of the standard.
@mickmis updated as discussed at the meeting (keep UAType, add VTOL to it, remove RIDAircraftType) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realized that there is one more difference for some fields between their v19 and v22a versions: their default values. Given that it is a (subtle) behavior change of an interface I think it's important to isolate that in a separate PR/commit with a version increment.
Example in this PR:
- old
HorizontalAccuracy
: v19, new: v22a - old
VerticalAccuracy
: v19, new: v22a - old and new
SpeedAccuracy
: v19
So I propose the following course of action:
- Update this PR that moves/add schemas so that there is no behavior change at all (i.e. keep the v19 versions, including
RIDAircraftType
as is) - Add a new PR migrating schemas to v22a (including
RIDAircraftType
->UAType
with the VTOL exception), there do a version increment
c9d5498
to
5f29951
Compare
… the following commits that update the version)
5f29951
to
4f712d5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM thanks
To prepare for some changes to the injection and observation API that is required for interuss/monitoring#754, this PR moves and adds some object definitions to commons.yaml, so they may be referenced from both injection.yaml and observation.yaml in subsequent PRs.
Definitions and behaviors should still be equivalent to before.