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.

Sign up for GitHub

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/scenarios/netrid/nominal_behavior] Add checks for UA type in SP (NET0260) #865

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+134 −6
Open

[uss_qualifier/scenarios/netrid/nominal_behavior] Add checks for UA type in SP (NET0260) #865

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 84 additions & 0 deletions monitoring/uss_qualifier/scenarios/astm/netrid/common_dictionary_evaluator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import datetime
import math
from typing import List, Optional

Expand Down Expand Up @@ -47,11 +48,20 @@ def __init__(

def evaluate_sp_flight(
self,
injected_flight: injection.TestFlight,
observed_flight: Flight,
participant_id: ParticipantID,
query_timestamp: datetime.datetime,
):
"""Implements fragment documented in `common_dictionary_evaluator_sp_flight.md`."""

self._evaluate_ua_type(
injected_flight.get("aircraft_type"),
observed_flight.aircraft_type,
[participant_id],
query_timestamp,
)

self._evaluate_operational_status(
observed_flight.operational_status,
[participant_id],
Expand Down Expand Up @@ -651,3 +661,77 @@ def _evaluate_operational_status(
key="skip_reason",
message=f"Unsupported version {self._rid_version}: skipping Operational Status evaluation",
)

def _evaluate_ua_type(
self,
injected_val: Optional[str],
observed_val: Optional[str],
participants: List[ParticipantID],
Copy link
Member

Choose a reason for hiding this comment

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

It would probably be helpful to clarify which participant(s) these are to make evaluation of the checks more straightforward -- perhaps "participant providing the API through which the value was observed" which would cover the SP-DP API in this PR and the Observation API in future PRs.

query_timestamp: datetime.datetime,
):
with self._test_scenario.check(
"UA type is present and consistent with injected one",
participants,
) as check:
if observed_val is None:
Copy link
Member

Choose a reason for hiding this comment

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

It seems like this is only valid if injected_val is not None. And, I'm not sure it would be wrong for a USS to report NotDeclared if injected_val was None.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The way I understand the standard is that the USS must always report the UA type because it is a required field, and if it is unknown (i.e. nothing was injected), it must report NotDeclared. Thus why I included this failure when it is not exposed.
However on the other side the observation/DP interface does not have marked as required the field aircraft_type, but the rid/SP interface does have it required.

I'm not sure how to proceed given that.
Shall we accept omission of that field?
Shall we discriminate between DP and SP for failing that check when field is not exposed?

Copy link
Member

Choose a reason for hiding this comment

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

Ah, my mistake -- this is the SP-DP interface and this is a required field, so you're right that None is simply invalid regardless of injection. None should not be allowed past the parser, but doesn't hurt to double check here.

The DP under test can't be at fault because uss_qualifier is acting as DP when querying the SP, so the DP under test isn't even involved.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

None should not be allowed past the parser

Indeed.

The DP under test can't be at fault because uss_qualifier is acting as DP when querying the SP, so the DP under test isn't even involved.

I'm not sure your comment was regarding that, but do note that the function _evaluate_ua_type implemented here is meant to be reused for both SP (this PR) and DP (PR #866). The caller does have to adapt the participants depending on which is tested.

Copy link
Member

Choose a reason for hiding this comment

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

observed_val being None is invalid for the SP API, but it's valid for the Observation API so I think this is good for this PR but possibly not for future ones.

check.record_failed(
"UA type is missing",
details="USS did not return any UA type",
query_timestamps=[query_timestamp],
)
elif not observed_val:
check.record_failed(
"UA type is empty",
details="USS returned an empty UA type",
query_timestamps=[query_timestamp],
)

equivalent = {injection.UAType.HybridLift, injection.UAType.VTOL}
if injected_val is None:
if observed_val != injection.UAType.NotDeclared:
check.record_failed(
Copy link
Member

Choose a reason for hiding this comment

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

I think this should actually raise an exception -- the injection attempt should be rejected if uss_qualifier doesn't follow the injection API and I think the injection API has the injected value as either omitted or a valid UAType, so there should be no way for us to reach this failure if uss_qualifier is working properly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK will change (and also take into account for future checks) that we should raise exceptions in cases where we injected invalid data.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually what I propose is to just adapt that check to fail when there is no injected value and that the observed value is different from NotDeclared. I'm not sure that we should start explicitly verifying the correctness of injected values (here we now don't need to try parsing injected_val anymore).

Copy link
Member

Choose a reason for hiding this comment

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

I think we want to fail as soon as possible whenever a contract is violated and injecting an invalid UAType violates the injection API contract. If we haven't already failed due to that contract failure, I think it's appropriate to fail here. Basically, my understanding is that we're evaluating a value in the SP or observation API in C1 or C2 below (where the test designer or uss_qualifier is at fault). Probably we shouldn't reach that point, but if we do, I don't think the failure responsibility somehow transfers to the SP or DP. I think the appropriate way to indicate a uss_qualifier or test design problem is to raise an exception.

The table below is my understanding of who is at fault for what failure, and the priority of rows goes from top to bottom. Note the difference between C3 and C8 when reusing the same function to evaluate SP API responses and Observation API responses.

C Data to inject Injection API SP API Observation API Responsible entity Failure
1 [invalid] [any] [any] [any] Test designer Invalid test data provided
2 [none or valid] [anything that doesn't match data to inject, including invalid] [any] [any] uss_qualifier developer Test data not injected accurately
3 [none or valid] [matching data to inject] [none] [any] SP API-required field not provided
4 [none] NotDeclared [any] [any] uss_qualifier developer Test data not injected accurately
5 [none or valid] [matching data to inject] [invalid] [any] SP SP API contract violated
6 [none] [none] [anything other than NotDeclared] [any] SP UA type not communicated correctly
7 [none or valid] [matching data to inject] [valid, but not corresponding to injected value] [any] SP UA type not communicated correctly
8 [none or valid] [matching injected data] [valid] [none] N/A No failure; reporting UA type to observers is not required by API
9 [none or valid] [matching injected data] [valid] [invalid] DP Observer API contract violated
10 [none or valid] [matching injected data] [valid] [valid, but not corresponding to SP value] DP SP information incorrectly reported to observer

"UA type is inconsistent, expected 'NotDeclared' since no value was injected",
details=f"USS returned the UA type {observed_val}, yet no value was injected, which should have been mapped to 'NotDeclared'.",
query_timestamps=[query_timestamp],
)

elif injected_val in equivalent:
if observed_val not in equivalent:
check.record_failed(
"UA type is inconsistent with injected value",
details=f"USS returned the UA type {observed_val}, yet the value {injected_val} was injected, given that {equivalent} are equivalent .",
query_timestamps=[query_timestamp],
)

elif injected_val != observed_val:
check.record_failed(
"UA type is inconsistent with injected value",
details=f"USS returned the UA type {observed_val}, yet the value {injected_val} was injected.",
query_timestamps=[query_timestamp],
)

with self._test_scenario.check(
"UA type is consistent with Common Data Dictionary",
participants,
) as check:
try:
injection.UAType(observed_val)
BenjaminPelletier marked this conversation as resolved.
Show resolved Hide resolved
except ValueError:
check.record_failed(
"UA type is invalid",
details=f"USS returned an invalid UA type: {observed_val}.",
query_timestamps=[query_timestamp],
)

if (
self._rid_version == RIDVersion.f3411_19
and observed_val == injection.UAType.HybridLift
) or (
self._rid_version == RIDVersion.f3411_22a
and observed_val == injection.UAType.VTOL
):
check.record_failed(
"UA type is inconsistent RID version",
details=f"USS returned the UA type {observed_val} which is not supported by the RID version used ({self._rid_version}).",
query_timestamps=[query_timestamp],
)
8 changes: 6 additions & 2 deletions monitoring/uss_qualifier/scenarios/astm/netrid/display_data_evaluator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import math
from dataclasses import dataclass
from typing import List, Optional, Dict, Union, Set, Tuple, cast
from typing import List, Optional, Dict, Union, Set, Tuple

import arrow
import s2sphere
Expand Down Expand Up @@ -890,6 +890,7 @@ def _evaluate_normal_sp_observation(

for mapping in mappings.values():
participant_id = mapping.injected_flight.uss_participant_id
injected_flight = mapping.injected_flight.flight
observed_flight = mapping.observed_flight.flight
flights_queries = [
q
Expand All @@ -901,6 +902,7 @@ def _evaluate_normal_sp_observation(
f"Found {len(flights_queries)} flights queries (instead of the expected 1) for flight {mapping.observed_flight.id} corresponding to injection ID {mapping.injected_flight.flight.injection_id} for {participant_id}"
)
flights_query = flights_queries[0]
query_timestamp = flights_query.query.request.timestamp

# Verify that flights queries returned correctly-formatted data
errors = schema_validation.validate(
Expand All @@ -920,7 +922,7 @@ def _evaluate_normal_sp_observation(
f"At {e.json_path} in the response: {e.message}"
for e in errors
),
query_timestamps=[flights_query.query.request.timestamp],
query_timestamps=[query_timestamp],
)

# Check recent positions timings
Expand All @@ -935,8 +937,10 @@ def _evaluate_normal_sp_observation(

# Check flight consistency with common data dictionary
self._common_dictionary_evaluator.evaluate_sp_flight(
injected_flight,
observed_flight,
participant_id,
query_timestamp,
)

# Check that required fields are present and match for any observed flights matching injected flights
Expand Down
10 changes: 10 additions & 0 deletions ...ss_qualifier/scenarios/astm/netrid/v19/common_dictionary_evaluator_sp_flight.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,16 @@

This fragment is implemented in `common_dictionary_evaluator.py:RIDCommonDictionaryEvaluator.evaluate_sp_flight`.

## ⚠️ UA type is present and consistent with injected one check

**[astm.f3411.v19.NET0260,Table1,3](../../../../requirements/astm/f3411/v19.md)** requires that relevant Remote ID data, consistent with the common data dictionary, be reported by the Service Provider.
The UA type being a required field, this check will fail if it is missing or if it inconsistent with the injected value.

## ⚠️ UA type is consistent with Common Data Dictionary check

**[astm.f3411.v19.NET0260,Table1,3](../../../../requirements/astm/f3411/v19.md)** requires that relevant Remote ID data, consistent with the common data dictionary, be reported by the Service Provider.
This check will fail if the observed UA type has an invalid value.

## Service Provider altitude check

**[astm.f3411.v19.NET0260,Table1,11](../../../../requirements/astm/f3411/v19.md)** requires that relevant Remote ID data, consistent with the common data dictionary, be reported by the Service Provider. Injected flight data had known altitudes, but the altitude reported by the Service Provider did not match those known altitudes.
Expand Down
10 changes: 10 additions & 0 deletions ...s_qualifier/scenarios/astm/netrid/v22a/common_dictionary_evaluator_sp_flight.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,16 @@

This fragment is implemented in `common_dictionary_evaluator.py:RIDCommonDictionaryEvaluator.evaluate_sp_flight`.

## ⚠️ UA type is present and consistent with injected one check

**[astm.f3411.v22a.NET0260,Table1,2](../../../../requirements/astm/f3411/v22a.md)** requires that relevant Remote ID data, consistent with the common data dictionary, be reported by the Service Provider.
The UA type being a required field, this check will fail if it is missing or if it inconsistent with the injected value.

## ⚠️ UA type is consistent with Common Data Dictionary check

**[astm.f3411.v22a.NET0260,Table1,2](../../../../requirements/astm/f3411/v22a.md)** requires that relevant Remote ID data, consistent with the common data dictionary, be reported by the Service Provider.
This check will fail if the observed UA type has an invalid value.

## Service Provider altitude check

**[astm.f3411.v22a.NET0260,Table1,12](../../../../requirements/astm/f3411/v22a.md)** requires that relevant Remote ID data, consistent with the common data dictionary, be reported by the Service Provider. Injected flight data had known altitudes, but the altitude reported by the Service Provider did not match those known altitudes.
Expand Down
7 changes: 6 additions & 1 deletion monitoring/uss_qualifier/suites/astm/netrid/f3411_19.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
<th><a href="../../README.md#checked-in">Checked in</a></th>
</tr>
<tr>
<td rowspan="75" style="vertical-align:top;"><a href="../../../requirements/astm/f3411/v19.md">astm<br>.f3411<br>.v19</a></td>
<td rowspan="76" style="vertical-align:top;"><a href="../../../requirements/astm/f3411/v19.md">astm<br>.f3411<br>.v19</a></td>
<td><a href="../../../requirements/astm/f3411/v19.md">DSS0010</a></td>
<td>Implemented</td>
<td><a href="../../../scenarios/astm/netrid/v19/dss/token_validation.md">ASTM NetRID DSS: Token Validation</a></td>
Expand Down Expand Up @@ -311,6 +311,11 @@
<td>Implemented</td>
<td><a href="../../../scenarios/astm/netrid/v19/nominal_behavior.md">ASTM NetRID nominal behavior</a></td>
</tr>
<tr>
<td><a href="../../../requirements/astm/f3411/v19.md">NET0260,Table1,3</a></td>
<td>Implemented</td>
<td><a href="../../../scenarios/astm/netrid/v19/nominal_behavior.md">ASTM NetRID nominal behavior</a></td>
</tr>
<tr>
<td><a href="../../../requirements/astm/f3411/v19.md">NET0260,Table1,5</a></td>
<td>Implemented</td>
Expand Down
7 changes: 6 additions & 1 deletion monitoring/uss_qualifier/suites/astm/netrid/f3411_22a.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
<th><a href="../../README.md#checked-in">Checked in</a></th>
</tr>
<tr>
<td rowspan="100" style="vertical-align:top;"><a href="../../../requirements/astm/f3411/v22a.md">astm<br>.f3411<br>.v22a</a></td>
<td rowspan="101" style="vertical-align:top;"><a href="../../../requirements/astm/f3411/v22a.md">astm<br>.f3411<br>.v22a</a></td>
<td><a href="../../../requirements/astm/f3411/v22a.md">DSS0010</a></td>
<td>Implemented</td>
<td><a href="../../../scenarios/astm/netrid/v22a/dss/token_validation.md">ASTM NetRID DSS: Token Validation</a></td>
Expand Down Expand Up @@ -321,6 +321,11 @@
<td>Implemented</td>
<td><a href="../../../scenarios/astm/netrid/v22a/nominal_behavior.md">ASTM NetRID nominal behavior</a></td>
</tr>
<tr>
<td><a href="../../../requirements/astm/f3411/v22a.md">NET0260,Table1,2</a></td>
<td>Implemented</td>
<td><a href="../../../scenarios/astm/netrid/v22a/nominal_behavior.md">ASTM NetRID nominal behavior</a></td>
</tr>
<tr>
<td><a href="../../../requirements/astm/f3411/v22a.md">NET0260,Table1,20</a></td>
<td>Implemented</td>
Expand Down
7 changes: 6 additions & 1 deletion monitoring/uss_qualifier/suites/uspace/network_identification.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
<th><a href="../README.md#checked-in">Checked in</a></th>
</tr>
<tr>
<td rowspan="100" style="vertical-align:top;"><a href="../../requirements/astm/f3411/v22a.md">astm<br>.f3411<br>.v22a</a></td>
<td rowspan="101" style="vertical-align:top;"><a href="../../requirements/astm/f3411/v22a.md">astm<br>.f3411<br>.v22a</a></td>
<td><a href="../../requirements/astm/f3411/v22a.md">DSS0010</a></td>
<td>Implemented</td>
<td><a href="../../scenarios/astm/netrid/v22a/dss/token_validation.md">ASTM NetRID DSS: Token Validation</a></td>
Expand Down Expand Up @@ -317,6 +317,11 @@
<td>Implemented</td>
<td><a href="../../scenarios/astm/netrid/v22a/nominal_behavior.md">ASTM NetRID nominal behavior</a></td>
</tr>
<tr>
<td><a href="../../requirements/astm/f3411/v22a.md">NET0260,Table1,2</a></td>
<td>Implemented</td>
<td><a href="../../scenarios/astm/netrid/v22a/nominal_behavior.md">ASTM NetRID nominal behavior</a></td>
</tr>
<tr>
<td><a href="../../requirements/astm/f3411/v22a.md">NET0260,Table1,20</a></td>
<td>Implemented</td>
Expand Down
7 changes: 6 additions & 1 deletion monitoring/uss_qualifier/suites/uspace/required_services.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
<th><a href="../README.md#checked-in">Checked in</a></th>
</tr>
<tr>
<td rowspan="100" style="vertical-align:top;"><a href="../../requirements/astm/f3411/v22a.md">astm<br>.f3411<br>.v22a</a></td>
<td rowspan="101" style="vertical-align:top;"><a href="../../requirements/astm/f3411/v22a.md">astm<br>.f3411<br>.v22a</a></td>
<td><a href="../../requirements/astm/f3411/v22a.md">DSS0010</a></td>
<td>Implemented</td>
<td><a href="../../scenarios/astm/netrid/v22a/dss/token_validation.md">ASTM NetRID DSS: Token Validation</a></td>
Expand Down Expand Up @@ -318,6 +318,11 @@
<td>Implemented</td>
<td><a href="../../scenarios/astm/netrid/v22a/nominal_behavior.md">ASTM NetRID nominal behavior</a></td>
</tr>
<tr>
<td><a href="../../requirements/astm/f3411/v22a.md">NET0260,Table1,2</a></td>
<td>Implemented</td>
<td><a href="../../scenarios/astm/netrid/v22a/nominal_behavior.md">ASTM NetRID nominal behavior</a></td>
</tr>
<tr>
<td><a href="../../requirements/astm/f3411/v22a.md">NET0260,Table1,20</a></td>
<td>Implemented</td>
Expand Down
Loading