fsfw/fsfw
fsfw
/
fsfw
10
3
Fork 6
Code Issues 31 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 4 Commits 19 Files Changed 34 +717 -75
meierj commented 2022-06-06 11:16:17 +02:00
Owner
Copy Link
  • 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
fsfw/fsfw/pipeline/head This commit looks good Details
951c077abc option to use Countdown object to time out replies
fsfw/fsfw/pipeline/head This commit looks good Details
0aee86442e typo in readme
fsfw/fsfw/pipeline/head This commit looks good Details
1611a4e1f0 device handler unittest wip
fsfw/fsfw/pipeline/head This commit looks good Details
161dbde0d7 fixed merge conflicts
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 unit test for detection of missed reply when commanded externally
meierj changed title from DHB Reply Timeout to WIP: DHB Reply Timeout 2022-06-06 11:17:13 +02:00
meierj added 1 commit 2022-06-06 11:59:49 +02:00
fsfw/fsfw/pipeline/pr-development There was a failure building this commit Details
ae2f7219fd run auto-formatter
meierj added 1 commit 2022-06-06 12:26:16 +02:00
fsfw/fsfw/pipeline/pr-development This commit looks good Details
103661facc deugging
meierj added 1 commit 2022-06-06 12:30:41 +02:00
fsfw/fsfw/pipeline/pr-development There was a failure building this commit Details
ade36e65c6 missed reply check in simple command nominal test case
meierj added 1 commit 2022-06-06 12:35:14 +02:00
fsfw/fsfw/pipeline/pr-development There was a failure building this commit Details
21eb386f3c changed reply timeouts
meierj changed title from WIP: DHB Reply Timeout to DHB Reply Timeout 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
mohr requested changes 2022-06-09 15:20:20 +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

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

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

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

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
fsfw/fsfw/pipeline/pr-development This commit looks good Details
3e9ae62b28 Merge branch 'development' into meier/dhbReplyTimeout
meierj added 1 commit 2022-06-23 11:55:15 +02:00
fsfw/fsfw/pipeline/pr-development This commit looks good Details
2d2f65bf89 moved activation of periodic replies to updatePeriodicReply
meierj added 1 commit 2022-06-23 11:57:03 +02:00
fsfw/fsfw/pipeline/pr-development This commit looks good Details
df97bbc691 run auto-formatter
mohr added 1 commit 2022-07-13 21:58:12 +02:00
fsfw/fsfw/pipeline/pr-development There was a failure building this commit Details
adbf375f38 some small fixes to dhb countdown addition
mohr commented 2022-07-13 21:59:29 +02:00
Owner
Copy Link

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

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
fsfw/fsfw/pipeline/pr-development This commit looks good Details
e8b8fff0b5 Merge branch 'development' into meier/dhbReplyTimeout
meierj added 1 commit 2022-07-14 09:15:26 +02:00
fsfw/fsfw/pipeline/pr-development This commit looks good Details
fsfw/fsfw/pipeline/head This commit looks good Details
ecac08814e better naming for functions which reset states of replies
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

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

  
bug

Something is not working

  
build

Issues related to the buildsystem, including CI

  
cosmetics

Small change most users won't notice

  
Documentation

Improvement of code documentation

  
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

  
Refactor

Contains a refactoring of some code

  
Tests

Issues related to testing code

  
wontfix

This won't be fixed

No Label
API Change
Breaking API Change
bug
build
cosmetics
Documentation
duplicate
feature
help wanted
hotfix
invalid
question
Refactor
Tests
wontfix
Milestone
Clear milestone
No items
No Milestone
v5.0.0
Assignees
Clear assignees
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: fsfw/fsfw#637
No description provided.
Delete Branch "meier/dhbReplyTimeout"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?