fsfw/fsfw
12
3 Fork 6
Code Issues 32 Pull Requests 4 Releases 10 Wiki Activity

DHB Reply Timeout #637

Merged
meierj merged 19 commits from meier/dhbReplyTimeout into development 2022-07-14 09:17:20 +02:00
Conversation 5 Commits 19 Files Changed 34
+717 -75
meierj commented 2022-06-06 11:16:17 +02:00
Owner
Copy link
Copy source
  • for some devices it might be convenient to timeout replies by using the time instead of the number of polling cycles
  • this extension allows to link a reply to a Countdown object instead of specifying delay cycles
  • added unit test to verify the detection of missed replies both for periodic and action replies
  • adaption in helper script to optionally specify the cmake generator
  • adaptions required for unit tests in destructor of HealthHelper and FailureIsolationBase
  • HealthHelper:
    • remove object from health table when device handler is deleted which happens after each test case
  • FailureIsolationBase:
    • unsubscribe from event range when device handler is deleted
* for some devices it might be convenient to timeout replies by using the time instead of the number of polling cycles * this extension allows to link a reply to a Countdown object instead of specifying delay cycles * added unit test to verify the detection of missed replies both for periodic and action replies * adaption in helper script to optionally specify the cmake generator * adaptions required for unit tests in destructor of HealthHelper and FailureIsolationBase * HealthHelper: * remove object from health table when device handler is deleted which happens after each test case * FailureIsolationBase: * unsubscribe from event range when device handler is deleted
meierj added 8 commits 2022-06-06 11:16:19 +02:00
option to use Countdown object to time out replies
fsfw/fsfw/pipeline/head This commit looks good
Details
951c077abc
typo in readme
fsfw/fsfw/pipeline/head This commit looks good
Details
0aee86442e
fixed merge conflict 7afe30ea88
device handler unittest wip
fsfw/fsfw/pipeline/head This commit looks good
Details
1611a4e1f0
unit test for dhb bf673c56c6
device fdir mock 2fa4fd61d0
fixed merge conflicts
fsfw/fsfw/pipeline/head This commit looks good
Details
161dbde0d7
unit test for detection of missed reply when commanded externally
fsfw/fsfw/pipeline/head There was a failure building this commit
Details
fsfw/fsfw/pipeline/pr-development There was a failure building this commit
Details
4fba2704aa
meierj marked the pull request as work in progress 2022-06-06 11:17:13 +02:00
meierj added 1 commit 2022-06-06 11:59:49 +02:00
run auto-formatter
fsfw/fsfw/pipeline/pr-development There was a failure building this commit
Details
ae2f7219fd
meierj added 1 commit 2022-06-06 12:26:16 +02:00
deugging
fsfw/fsfw/pipeline/pr-development This commit looks good
Details
103661facc
meierj added 1 commit 2022-06-06 12:30:41 +02:00
missed reply check in simple command nominal test case
fsfw/fsfw/pipeline/pr-development There was a failure building this commit
Details
ade36e65c6
meierj added 1 commit 2022-06-06 12:35:14 +02:00
changed reply timeouts
fsfw/fsfw/pipeline/pr-development There was a failure building this commit
Details
21eb386f3c
meierj marked the pull request as ready for review 2022-06-06 12:37:12 +02:00
meierj requested review from gaisser 2022-06-06 12:37:20 +02:00
meierj requested review from mohr 2022-06-06 12:37:52 +02:00
src/fsfw/devicehandlers/DeviceHandlerBase.cpp Outdated
@@ -431,1 +447,4 @@
info.command = deviceCommandMap.end();
info.countdown = countdown;
if (info.periodic) {
info.active = true;
mohr commented 2022-06-09 15:20:03 +02:00
Owner
Copy link
Copy source

So far, convention is that periodic packets are disabled by default and need to be exlpicitely enabled. See info.delayCycles = 0; above.

So far, convention is that periodic packets are disabled by default and need to be exlpicitely enabled. See ` info.delayCycles = 0;` above.
mohr marked this conversation as resolved
src/fsfw/devicehandlers/DeviceHandlerBase.cpp Outdated
@@ -962,6 +1002,10 @@ ReturnValue_t DeviceHandlerBase::enableReplyInReplyMap(DeviceCommandMap::iterato
info->delayCycles = info->maxDelayCycles;
mohr commented 2022-06-09 15:19:54 +02:00
Owner
Copy link
Copy source

please add a comment, that both delay Cycles as well as the countdown are set if a countdown is preset, but the countdown will take precedence.

please add a comment, that both delay Cycles as well as the countdown are set if a countdown is preset, but the countdown will take precedence.
mohr marked this conversation as resolved
src/fsfw/devicehandlers/DeviceHandlerBase.h
@@ -448,6 +448,9 @@ class DeviceHandlerBase : public DeviceHandlerIF,
* by the device repeatedly without request) or not. Default is aperiodic (0).
* Please note that periodic replies are disabled by default. You can enable them with
* #updatePeriodicReply
mohr commented 2022-06-09 15:20:17 +02:00
Owner
Copy link
Copy source

updatePeriodicReply needs to be adapted, too.

`updatePeriodicReply` needs to be adapted, too.
mohr marked this conversation as resolved
mohr commented 2022-06-09 15:26:10 +02:00
Owner
Copy link
Copy source

Also, dude, you wrote a DHB unit test (add exploding head emoji, it seems gitea can not handle unicode..)

Also, dude, you wrote a DHB unit test (add exploding head emoji, it seems gitea can not handle unicode..)
muellerr added this to the v5.0.0 milestone 2022-06-20 16:16:44 +02:00
muellerr added 1 commit 2022-06-21 10:49:09 +02:00
Merge branch 'development' into meier/dhbReplyTimeout
fsfw/fsfw/pipeline/pr-development This commit looks good
Details
3e9ae62b28
meierj added 1 commit 2022-06-23 11:55:15 +02:00
moved activation of periodic replies to updatePeriodicReply
fsfw/fsfw/pipeline/pr-development This commit looks good
Details
2d2f65bf89
meierj added 1 commit 2022-06-23 11:57:03 +02:00
run auto-formatter
fsfw/fsfw/pipeline/pr-development This commit looks good
Details
df97bbc691
mohr added 1 commit 2022-07-13 21:58:12 +02:00
some small fixes to dhb countdown addition
fsfw/fsfw/pipeline/pr-development There was a failure building this commit
Details
adbf375f38
mohr commented 2022-07-13 21:59:29 +02:00
Owner
Copy link
Copy source

As I think it is faster than writing about it, I took the liberty to add my remaining comments directly as code.

@meierj if you agree with my changes, feel free to add a positive review and merge this PR.

As I think it is faster than writing about it, I took the liberty to add my remaining comments directly as code. @meierj if you agree with my changes, feel free to add a positive review and merge this PR.
mohr commented 2022-07-13 22:00:27 +02:00
Owner
Copy link
Copy source

Also, I think we should remove the delay based solution completely some time in the future. The countdown based one is nicer and having both gives not exactly beautiful code.

Also, I think we should remove the delay based solution completely some time in the future. The countdown based one is nicer and having both gives not exactly beautiful code.
mohr removed review request for gaisser 2022-07-13 22:01:23 +02:00
meierj added 2 commits 2022-07-14 09:01:44 +02:00
missing reset of timedOut value in loop of decrementDeviceReplyMap 4d34f93cfc
Merge branch 'development' into meier/dhbReplyTimeout
fsfw/fsfw/pipeline/pr-development This commit looks good
Details
e8b8fff0b5
meierj added 1 commit 2022-07-14 09:15:26 +02:00
better naming for functions which reset states of replies
fsfw/fsfw/pipeline/pr-development This commit looks good
Details
fsfw/fsfw/pipeline/head This commit looks good
Details
ecac08814e
meierj merged commit 3686bbc486 into development 2022-07-14 09:17:20 +02:00
muellerr deleted branch meier/dhbReplyTimeout 2022-07-14 13:19:13 +02:00
muellerr commented 2022-07-14 13:19:37 +02:00
Owner
Copy link
Copy source

I am working on refactoring the command/reply API anyway, it has become bloated up a bit.

I am working on refactoring the command/reply API anyway, it has become bloated up a bit.
Sign in to join this conversation.
Reviewers
No Reviewers
mohr
Labels
Clear labels
API Change
Changes the API, so Users need to adapted
 Breaking API Change
Changes the API in a non backwards compatible way
 Documentation
Improvement of code documentation
 Refactor
Contains a refactoring of some code
 Tests
Issues related to testing code
 bug
Something is not working
 build
Issues related to the buildsystem, including CI
 cosmetics
Small change most users won't notice
 duplicate
This issue or pull request already exists
 feature
New feature
 help wanted
Need some help
 hotfix
Quick or important fix that is merged directly into master, causing a new revision
 invalid
#worksforme
 question
More information is needed
 wontfix
This won't be fixed
No labels
Milestone
No items
No Milestone v5.0.0
Assignees
Clear assignees
ahinkel (Andy Hinkel) gaisser (Steffen Gaisser) jenkins_buildfix meierj (Jakob Meier) mohr (Ulrich Mohr) mspahr muellerr (Robin Müller) tbaumgartl (Tobias Baumgartl)
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fsfw/fsfw#637