-
Notifications
You must be signed in to change notification settings - Fork 20
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
[uss_qualifier] netrid: net0260 check observed height #840
Conversation
32cd7e7
to
4190368
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.
PR title mentions DSS0130 and code seems to test for SP requirement NET0260?
That is incorrect indeed. Fixed. |
07218b1
to
48b160f
Compare
@@ -49,6 +51,9 @@ def _rect_str(rect) -> str: | |||
|
|||
VERTICAL_SPEED_PRECISION = 0.1 | |||
SPEED_PRECISION = 0.05 | |||
HEIGHT_TOLERANCE_M = 1 | |||
# 20km above ground sounds like a reasonable maximum altitude before declaring it as an error |
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.
Is there anything saying so in the standard or OpenAPI? Otherwise I suggest sticking to checking if the value matches what was injected. I don't believe having an height of 25km actually contradicts anything in the standard. Maybe physics but some simulated data could be perfectly valid.
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.
This is for covering the case where a USS would decide to change the reference that is used: Ie, we may inject something that is relative to the operator, and the USS decides to convert this to height above ground.
I remember discussing at a meeting that we would not consider this as a violation and simply set some reasonable bounds, but I might have misunderstood?
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.
If we expect to find what we injected, we actually don't need this so the problem solves itself.
# If the reported height has a different type, we will check for absurd values below | ||
|
||
if ( | ||
injected_position.height is None |
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.
If we did not inject height but the SP reports some (valid one that is, not e.g. -1000m) that would be a reason for failing a check here.
or injected_position.height.reference.value | ||
!= observed_position.height.reference.value | ||
): | ||
# If the injected data does not specify a height or specifies a different type than the observed one, |
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.
Actually here following the clarification with @BenjaminPelletier last Tuesday, if the SP reports a height reference different than the one injected, we should fail the check, since IMO this is analog to the speed / speed accuracy discussion.
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.
Indeed, that would match the understanding I got from the last meeting, but contradicts a conclusion I remembered from an earlier meeting (that was specifically about height).
I'll go with the stricter interpretation and expect the observation to be equal to the injected value and we'll revisit if/when required
ceef7e7
to
725528c
Compare
9a3411e
to
60ed437
Compare
# We injected a height so expect to observe one | ||
if "height" not in observed_position: | ||
check.record_failed( | ||
"A value was injected for the height field, but none was returned in Service Provider response", |
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.
Include details like the other checks?
60ed437
to
4d6644b
Compare
First simple approach for validating height reported by the SP.
We could narrow down the checks by having test designers provide a resource specifying a maximum envelope for the height, but this is of lower priority and will be added later if time permits.
Part of the effort on #754