Feature/flight states #64
Conversation
cjhr95
added
documentation
flight/state_settings.py
Outdated
| Methods | ||
| ------- | ||
| __init__ | ||
| Sets preliminary values for SUAS overheads | ||
| @Property | ||
| simple_takeoff() -> bool | ||
| Setter to determine if testing takeoff procedure desired | ||
| num_waypoints() -> int | ||
| Sets the number of waypoints in the competition | ||
| run_title() -> str | ||
| Sets the name of the current flight for logging | ||
| run_description() -> str | ||
| Sets the description of current flight mission | ||
|
|
||
| @Setters | ||
| simple_takeoff() -> None | ||
| Enables access to status of simple_takeoff | ||
| num_waypoints() -> None | ||
| Enables access to number of waypoints in the competition | ||
| run_title() -> None | ||
| Enables access to description of current flight mission | ||
| run_description() -> None | ||
| Enables access to description of current flight mission | ||
|
|
||
| Attributes | ||
| ---------- | ||
| __num_waypoints: int | ||
| Number of waypoints on flight plan | ||
| __run_title: str | ||
| Title of Competition | ||
| __run_description: str | ||
| Description for Competition |
There was a problem hiding this comment.
Attributes section should come first in the class docstring, and it shouldn't list private variables, but rather should list the variables as their accessed, so its list would be what you have just without the leading __ because that's how the instance variables are accessed.
Methods section should not include getters/setters because those functions should never be called explicitly (that's the point of the @Property and @property_name.setter decorators.
For example an instance setting of StateSettings can just set __num_waypoints by doing setting.num_waypoints = ...
flight/states/land.py
Outdated
| Parameters | ||
| ---------- | ||
| run() -> Final | ||
| Running the landing procedure after returning to home |
There was a problem hiding this comment.
Did you mean for this section to be titled "Methods"?
flight/states/air_drop.py
Outdated
|
|
||
| Attributes | ||
| ---------- | ||
| N/A |
flight/states/emergent_odlc.py
Outdated
| Attributes | ||
| ---------- | ||
| N/A |
There was a problem hiding this comment.
Same as above, should be None instead of N/A
flight/util/json_parsing.py
Outdated
| f.close() | ||
| f.close() |
There was a problem hiding this comment.
You don't need an f.close since you're using a context manager.
Also everything should be either structurally subtyped or Any, so you can either replace List with List[Any] if you're sure that the json is to be loaded in as a dictionary mapping strings to lists or you can replace List with just Any so that you can account for any type being mapped. Json typing is really not in a good place right now in python, so that's the only reasonably readable way to do it.
The result should look like this:
with open(filename) as f:
data_set: Dict[str, Any] = json.load(f)
flight/util/json_parsing.py
Outdated
| Raises | ||
| ------ | ||
| General | ||
| Designed to detect any error to prevent data corruption and always close the file being read |
There was a problem hiding this comment.
Same as above, this section is not needed because nothing is being raised with the raise keyword
flight/util/json_parsing.py
Outdated
| Raises | ||
| ------ | ||
| General | ||
| Designed to detect any error to prevent data corruption and always close the file being read |
There was a problem hiding this comment.
This doesn't actually raise this.
Instead you're technically "catching" an exception, which is not something you need to mention in the docstring.
flight/util/json_parsing.py
Outdated
| with open(filename) as f: | ||
| try: | ||
| data_set: Dict[str, List] = json.load(f) | ||
| except: | ||
| f.close() | ||
| f.close() |
There was a problem hiding this comment.
Same as above, context managers handle file closing, so you don't need any of the try-except or f.close() stuff
flight/util/json_parsing.py
Outdated
|
|
||
| Returns | ||
| ------- | ||
| List[Dict[str, float]] |
There was a problem hiding this comment.
Since you're returning a variable name, I'd take the opportunity to include that in here.
This would look like this (and this is what the colon is meant for).
stationary_obs : List[Dict[str, float]]instead of just List[Dict[str, float]]
| from states.state import State | ||
| from states.start_state import Start | ||
| from states.pre_processing import PreProcess | ||
| from states.takeoff import Takeoff | ||
| from states.waypoints import Waypoints | ||
| from states.land import Land | ||
| from states.final_state import Final |
There was a problem hiding this comment.
Again, these need to be absolute imports.
Ex. from flight.states.state import State
|
|
||
| Returns | ||
| ------- | ||
| Land |
|
|
||
| Methods | ||
| ------- | ||
| run() -> Land |
There was a problem hiding this comment.
Did you mean run() -> OffAxisODLC?
| """ | ||
| return | ||
|
|
||
| async def _check_arm_or_arm(self, drone: System) -> None: |
There was a problem hiding this comment.
Name this a little less redundantly.
Maybe something like _ensure_arm()
Better flight states, along with JSON parsing.