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.

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

creating "safe mode" #2888

Merged
merged 5 commits into from
Sep 17, 2024
Merged

Conversation

Williangalvani
Copy link
Member

@Williangalvani Williangalvani commented Sep 9, 2024

Screenshot 2024-09-09 at 12 18 49

helps #2752

Comment on lines +58 to +62
get is_safe() {
// We can potentially check for external things here
return !this.verhicle_armed
}

Copy link
Member

Choose a reason for hiding this comment

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

why is this not calling the backend instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

This will give us feedback at 1hz, I don't want yet another thing polling the backends at 1hz for no reason...
I can add to the comment "such as ardupilotManager's /safe endpoint"

Copy link
Member

Choose a reason for hiding this comment

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

So why we have the safe endpoint in the backend ?

Copy link
Member

Choose a reason for hiding this comment

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

hmm, so why was the backend endpoint added?

Copy link
Member Author

Choose a reason for hiding this comment

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

for subsequent backend-based validations, such in ardupilot_manager.py and whatever else

@@ -290,6 +290,12 @@ async def available_boards() -> Any:
return await autopilot.available_boards(True)


@app.get("/safe")
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't make more sense to move to "armed" ? Safe is to abstracted, and armed is a common term.

Copy link
Member

Choose a reason for hiding this comment

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

@patrickelectric I think safe can abstract more than just arm, in case we want to check for other things..?

Copy link
Member

Choose a reason for hiding this comment

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

any idea of other things that the backend of ardupilot_manager may know ?
It would be nice to have a comment about that.

Copy link
Member Author

Choose a reason for hiding this comment

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

if there's a flashing operation in progress, for example, I'll add comments and/or more checking

Copy link
Member Author

Choose a reason for hiding this comment

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

comment added, I'll leave the actual blocking checks to after @JoaoMario109 's refactor is done

@patrickelectric patrickelectric merged commit dfbb6ca into bluerobotics:master Sep 17, 2024
6 checks passed
@patrickelectric patrickelectric deleted the safe branch September 17, 2024 17:47
@ES-Alexander ES-Alexander added the docs-needed Change needs to be documented label Oct 7, 2024
@ES-Alexander ES-Alexander added docs-complete Change documentation has been completed and removed docs-needed Change needs to be documented labels Dec 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs-complete Change documentation has been completed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants