DHB refactor TM handler #669

Closed

muellerr wants to merge 10 commits from mueller/dhb-handle-device-tm into development

pull from: mueller/dhb-handle-device-tm

merge into: fsfw:development

fsfw:master

fsfw:development

fsfw:mohr/romeo

fsfw:development-doc-deployment

fsfw:development-doc-test0

fsfw:switch-doc-theme-to-rtd

fsfw:action-update

fsfw:obj-manager-tweak

fsfw:mohr_introspection

fsfw:mohr/windows

fsfw:mohr/GCC13

fsfw:mohr/timetag_fix

fsfw:mohr/relsease_helper

fsfw:mohr/warnings

fsfw:mohr/rtems

fsfw:mohr/freeRTOS

fsfw:mueller/new-cfdp-update-with-handlers

fsfw:mohr/dhb2normal

fsfw:mueller/obj-man-remove-weird-proc-func

fsfw:mohr/documentation_ci

fsfw:mueller/fsfw-from-zero

fsfw:mueller/data-wrapper

fsfw:mueller/refactor-tmtc-stack-with-retval-refactoring

fsfw:mueller/refactor-logging-with-fmt

fsfw:meier/upstream-pus-distributor-bugfix

fsfw:mueller/clang-improvements

Conversation 4 Commits 10 Files Changed 9 +204 -47

muellerr commented 2022-08-30 13:40:49 +02:00

Owner

Copy Link

• TM handler of DHB is a bit more flexible and can now handle raw buffers in addition to a packet implementing SerializeIF
• Refactor DeviceTmReportingWrapper to be able to use the new util::DataWrapper tagged union
• DeviceTmReportingWrapper deserialize is never used and is kind of tricky to implement now because one would have to pass the length of the expected data. I think the easier solution for now is to just forbid deserialization
• I tested this change on the EIVE fork. We have the case that we forward some TM to in raw format directly

- TM handler of DHB is a bit more flexible and can now handle raw buffers in addition to a packet implementing `SerializeIF` - Refactor `DeviceTmReportingWrapper` to be able to use the new `util::DataWrapper` tagged union - `DeviceTmReportingWrapper` deserialize is never used and is kind of tricky to implement now because one would have to pass the length of the expected data. I think the easier solution for now is to just forbid deserialization - I tested this change on the EIVE fork. We have the case that we forward some TM to in raw format directly

muellerr added 3 commits 2022-08-30 13:40:51 +02:00

add new data wrapper helper type 20d42add03

extend data wrapper 0f27c7e7e7

refactored dhb TM handler 3a47062f2a

gaisser was assigned by muellerr 2022-08-30 13:40:58 +02:00

muellerr added this to the v6.0.0 milestone 2022-08-30 13:41:00 +02:00

gaisser was unassigned by muellerr 2022-08-30 13:41:05 +02:00

muellerr added the

feature

label 2022-08-30 13:41:10 +02:00

muellerr added a new dependency 2022-08-30 13:41:16 +02:00

#668 add new data wrapper helper type

muellerr requested review from gaisser 2022-08-30 13:44:01 +02:00

muellerr requested review from mohr 2022-08-30 13:44:07 +02:00

muellerr added 4 commits 2022-08-30 14:03:45 +02:00

data wrapper update c756297e5c

basic data wrapper unittests bdd79d060d

additional test 192255df1c

Merge branch 'mueller/data-wrapper' into mueller/dhb-handle-device-tm 093052604a

muellerr added 1 commit 2022-08-30 14:40:01 +02:00

update changelog 6b8c83be29

muellerr added 2 commits 2022-08-30 14:42:12 +02:00

update changelog d675a789a2

Merge branch 'mueller/data-wrapper' into mueller/dhb-handle-device-tm e10e71cee9

muellerr added the

Breaking API Change

label 2022-08-30 14:43:43 +02:00

muellerr added 2 commits 2022-08-30 16:03:12 +02:00

additional tests 8d1777fa0c

Merge branch 'mueller/data-wrapper' into mueller/dhb-handle-device-tm fcdb90ff0a

muellerr added 1 commit 2022-08-30 16:04:20 +02:00

remove newline 5ff28ff562

mohr commented 2022-08-30 18:23:51 +02:00

Owner

Copy Link

I am not sure if I completely grasp the scope of this PR but following some thoughts:

• SerializeIF is our generic interface for passing data. Buffer/Size combinations are a subset of it via SerialBufferAdapter.

• For sending raw, uninterpreted data to ground within a DHB class, one can use DeviceHandlerBase::replyRawReplyIfnotWiretapped(), which has the benefit of sending it via special PUS Subservice, which lets the MCS detect that it is raw data. replyRawReplyIfnotWiretapped() is to be preferred over replyRawData() as it checks if wiretapping is active to avoid duplicate packets.

I am not sure if I completely grasp the scope of this PR but following some thoughts: - `SerializeIF` *is* our generic interface for passing data. Buffer/Size combinations are a subset of it via `SerialBufferAdapter`. - For sending raw, uninterpreted data to ground within a DHB class, one can use `DeviceHandlerBase::replyRawReplyIfnotWiretapped()`, which has the benefit of sending it via special PUS Subservice, which lets the MCS detect that it is raw data. `replyRawReplyIfnotWiretapped()` is to be preferred over `replyRawData()` as it checks if wiretapping is active to avoid duplicate packets.

mohr referenced this pull request 2022-08-30 18:25:57 +02:00

add new data wrapper helper type #668

muellerr commented 2022-08-30 19:11:11 +02:00

Author

Owner

Copy Link

I needed this after processing a data reply coming from a service 8 request. This was the most intuitive solution for me after having seen how handleDeviceTm was used at other places inside the EIVE code. It was used to package data replies for service 8 request. The only addition I really needed was to be able to supply raw buffers, so I would not have to create a dummy class just to wrap the raw pointer.

I needed this after processing a data reply coming from a service 8 request. This was the most intuitive solution for me after having seen how `handleDeviceTm` was used at other places inside the EIVE code. It was used to package data replies for service 8 request. The only addition I really needed was to be able to supply raw buffers, so I would not have to create a dummy class just to wrap the raw pointer.

muellerr commented 2022-08-30 19:19:04 +02:00

Author

Owner

Copy Link

One other way: Supply a second variant of handleDeviceTm which takes a raw pointer and a size and converts them to SerializeIF using the SerialBufferAdapter. See #671

One other way: Supply a second variant of `handleDeviceTm` which takes a raw pointer and a size and converts them to SerializeIF using the `SerialBufferAdapter`. See https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/671

mohr commented 2022-09-01 14:13:56 +02:00

Owner

Copy Link

Superseded by #671

Superseded by #671

mohr removed a dependency 2022-09-01 14:14:16 +02:00

#668 add new data wrapper helper type

mohr closed this pull request 2022-09-01 14:14:26 +02:00

mohr removed this from the v6.0.0 milestone 2023-02-09 16:07:33 +01:00

Some checks are pending

Hide all checks

fsfw/fsfw/pipeline/pr-development Build queued...

Details

fsfw/fsfw/pipeline/head This commit looks good

Details

Pull request closed

Please reopen this pull request to perform a merge.

Sign in to join this conversation.

Reviewers

No reviewers

gaisser

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

Assignees

Clear assignees

No Assignees

2 Participants

Notifications

Subscribe

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#669

No description provided.