-
Notifications
You must be signed in to change notification settings - Fork 55
Add Safety README #436
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
base: main
Are you sure you want to change the base?
Add Safety README #436
Conversation
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.
Can you please add a with context manager to the safety module?
The rest of the documentation looks good!
|
||
For example: | ||
|
||
```python |
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.
Missing indentation
### Safety Execption Classes | ||
|
||
- **`ThermalLimitException`**: Raised when software thermal limits are exceeded | ||
- **`I2tLimitException`**: Raised when Dephy ActPack I²t (current-time) limits are exceeded |
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 I2tlimitException is not limited to Dephy Actpacks
|
||
### [DephyActuator](./dephy.py) Safety Support | ||
|
||
#### Built-in Safety Features |
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 sub-section seems out of place and is not true
- Thermal scaling factor calculation | ||
- Integrated exception handling in `update()` method | ||
|
||
#### Known Issues |
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.
Same here, this does not belong in safety modules doc
|
||
### Best Practices | ||
|
||
1. **Always use context managers** (`with` statements) when working with actuators |
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 point outlines general safety but does not relate to our safety module implementation
""" | ||
return SafetyManager.SafetyDisableContext(self) | ||
|
||
class SafetyDisableContext: |
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.
Instead of a context manager to temporarily disable safety checks, I think it would be more useful to add the with context manager, where the enter(): function will call the safety manager start function and the exit(): function will execute the safety manager stop and exit conditions.
This will help us to use the safety manager with "with" similar to actuators.
Overall, the safety documentation looks good! great work!
Create a README for the safety module. Others can learn how to integrate the safety module into the actuators code.