[uss_qualifier] Use FlightInfoTemplate for flight intent storage #276
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mickmis
approved these changes
Oct 24, 2023
| volumes = [] | ||
| else: | ||
| raise ValueError( | ||
| f"Unrecongized uas_state '{info.basic_information.uas_state}'" |
There was a problem hiding this comment.
Suggested change
| f"Unrecongized uas_state '{info.basic_information.uas_state}'" | |
| f"Unrecognized uas_state '{info.basic_information.uas_state}'" |
| result = ImplicitDict.parse(base_object, type(base_object)) | ||
| if isinstance(base_object, dict): | ||
| result = {k: v for k, v in base_object.items()} | ||
| elif base_object is None: |
There was a problem hiding this comment.
This is never true because of
if base_object is None:
return overridesat the beginning of the function.
Or did you mean the following?
Suggested change
| elif base_object is None: | |
| elif overrides is None: |
But if so, could that be true? I.e. can both isinstance(overrides, dict) and overrides is None be true at the same time?
But IIUC the condition+return can just be removed and that's not an issue.
There was a problem hiding this comment.
I agree -- I think the issue I was trying to address was separate from this so I've removed the clause
Comment on lines
+79
to
+80
| test scenario. Additionally, their end time must leave sufficient time for the execution of the test scenario. For the | ||
| sake of simplicity, it is recommended to set the start and end times of all the intents to the same range. |
There was a problem hiding this comment.
This change should probably be also done on:
- monitoring/uss_qualifier/scenarios/astm/utm/nominal_planning/conflict_higher_priority/conflict_higher_priority.md
- monitoring/uss_qualifier/scenarios/astm/utm/flight_intent_validation/flight_intent_validation.md
There was a problem hiding this comment.
Sounds good -- done
github-actions bot
added a commit
that referenced
this pull request
Oct 24, 2023
* Use FlightInfoTemplate for flight intent storage * Address comments a09aac7
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR continues the transition away from the legacy scd flight planning API by changing the flight intent storage format from being based on InjectFlightRequest (scd API) to being based on FlightInfoTemplate. The CHE flight intent test data is updated to this new format, and the vertices are shortened to just a bounding box (rather than rounded-edge rectangles) for easier readability. The Kentland flight data was unused, so therefore it is deleted in this PR to reduce maintenance burden.
To allow reuse, resolving a FlightIntentCollection (with delta definitions) to a concrete collection is moved to the FlightIntentCollection object itself.
Because verifying the new flight intent data was hard, a small script to render a FlightIntentCollection as a Google Earth KML is added to the test_data folder.
The wrong Polygon type was being referenced in geotemporal, so this is fixed.