Repository navigation
HealthMonitor: Component requirements and AoU - #863
pawelrutkaq wants to merge 2 commits into
Conversation
GitHub Actions
Error
GitHub Actions
Error
GitHub Actions
Error
GitHub Actions
Error
|
Documentation preview for this pull request is available at: |
32c4b82 to
5f49358
Compare
GitHub Actions
Error
GitHub Actions
Error
GitHub Actions
Error
GitHub Actions
Error
| monitoring instance if any of the following holds: | ||
|
|
||
| - no monitor is configured, | ||
| - the supervisor notification cycle is not an integer multiple of the |
There was a problem hiding this comment.
Those "config validation" reqs are assertions turned into requirements.
Rephrase to disallow lack of configured monitors, drop the rest.
There was a problem hiding this comment.
Subject to second opinion if this is too specific
| :status: valid | ||
| :version: 1 | ||
|
|
||
| The :term:`Health Monitor` shall not accept a time range whose minimum is |
There was a problem hiding this comment.
Assertion turned into requirement, not sure how helpful it is.
There was a problem hiding this comment.
Second opinion whether its too specific for comp req; however, this is external input so to me souds like it shall be here
| :status: valid | ||
| :version: 1 | ||
|
|
||
| The :term:`Health Monitor` shall reject a heartbeat monitor configuration if |
There was a problem hiding this comment.
Not a requirement, it's a constraint due to implementation detail.
There was a problem hiding this comment.
Yes, however, it still sounds valid, we need a second opinion ;)
|
|
||
| Deadline Supervision | ||
| ==================== | ||
|
|
There was a problem hiding this comment.
Is a deadline monitor without configured deadlines valid?
| - Set the status to valid and start the review/merge process | ||
| - Add other needed requirements for your feature | ||
| .. note:: Kept until the architecture elements are relinked to the | ||
| requirements above. See https://github.com/eclipse-score/lifecycle/issues/366. |
| :status: valid | ||
| :version: 1 | ||
|
|
||
| The process that hosts the :term:`Health Monitor` shall be developed to |
There was a problem hiding this comment.
Could You elaborate? Why does it matter, especially in regard to the note?
There was a problem hiding this comment.
This is AoU so You cannot use HM in ASIL D, But to increase reliability and due to the process nature in POSIX, the hosted code in same process shall have same required safty level. Anyway AoU can be discarded with proper evidence by user. Open to second opinion if we need this
| the internal evaluation cycle is not shorter than twice the minimum of the | ||
| heartbeat time range. |
There was a problem hiding this comment.
not shorter I think its not aligned with impl as code allows <=
There was a problem hiding this comment.
I think you read condition wrongly but I reworded double negation in this req
| The :term:`Health Monitor` shall reject a configuration and not create a | ||
| monitoring instance if any of the following holds: |
There was a problem hiding this comment.
what does it mean reject? error? empty instance?
There was a problem hiding this comment.
This would be too detailed imho, but generally shall not create an instance, so error (it clearly says not create too or ?)
| :version: 1 | ||
|
|
||
| The reporting operations of the :term:`Health Monitor` (heartbeat, deadline | ||
| start and stop, state transition) shall have a bounded execution time, and |
There was a problem hiding this comment.
imho comp req should be more strict - what is min, max of those boundaries?
There was a problem hiding this comment.
I don't think you can realistically set that, it depends on HW, on integration thread settings, and many more. The point is that this shall not be coupled with some generally random timing facilities (ie memory allocation).
Maybe thi shall not be even said in requrements, I would ask for Second Opinion
| :status: valid | ||
| :version: 1 | ||
|
|
||
| On any internal error, the :term:`Health Monitor` shall stop sending alive |
There was a problem hiding this comment.
do we have a list of all possible internal errors?
There was a problem hiding this comment.
No, it would not be feasible.
| The :term:`Health Monitor` shall provide a Rust API and a C++ API with | ||
| equivalent supervision semantics. |
There was a problem hiding this comment.
isnt it more like feat requirement?
There was a problem hiding this comment.
makse sense, done
GitHub Actions
Waiting
GitHub Actions
Waiting
GitHub Actions
Waiting
GitHub Actions
Waiting
|
@aschemmel-tech, we need your second opinion on comments with "second opinion" included (+ ofc review of all) ;) |
HealthMonitor: Component requirements and AoU