Possible bugfix in DHB #469

muellerr · 2021-09-06T12:10:00+02:00

muellerr commented

2021-09-06 12:10:00 +02:00

The delayCycles variables needs to be initialized differently
for periodic replies. It is initialized to the maxDelayCycles value now.

enableReplyInReplyMap was not called in my specific use case because no commands were being sent. delayCycles needs to be initiaited to a non-zero value in the case of a periodic reply because otherwise,the wrong branch in enableReply will be taken (see line ~800).

The delayCycles variables needs to be initialized differently for periodic replies. It is initialized to the `maxDelayCycles` value now. `enableReplyInReplyMap` was not called in my specific use case because no commands were being sent. delayCycles needs to be initiaited to a non-zero value in the case of a periodic reply because otherwise,the wrong branch in `enableReply` will be taken (see line ~800).

muellerr added 1 commit 2021-09-06 12:10:05 +02:00

Possible bugfix in DHB a6d744c9c8

The delayCycles variables needs to be initialized differently
for periodic replies.
It is initialized to the maxDelayCycles value now

muellerr added 1 commit 2021-09-06 12:10:28 +02:00

Merge remote-tracking branch 'upstream/development' into mueller/dhb-periodoc-reply-fix 7bf66dc401

muellerr added the

bug

label 2021-09-06 12:11:29 +02:00

muellerr added this to the v2.0.0 milestone 2021-09-06 18:48:44 +02:00

muellerr added 1 commit 2021-09-06 18:52:00 +02:00

Merge branch 'development' into mueller/dhb-periodoc-reply-fix 0a56441510

mohr commented

2021-09-10 12:55:32 +02:00

The intended design was that periodic replies could be enabled and disabled.
insertInReplyMap() is only the definition of the reply and should not enable it directly.
Enabling is performed by calling updateReplyMapEntry() and thus setting delayCycles.
If you would like to make this a bit clearer, there could be an [en|dis]ablePeriodicReply which has less parameters than updateReplyMapEntry(). Prbably none. It only verifies that the reply is indeed periodic and then sets the delayCycles.

The intended design was that periodic replies could be enabled and disabled. `insertInReplyMap()` is only the definition of the reply and should not enable it directly. Enabling is performed by calling `updateReplyMapEntry()` and thus setting `delayCycles`. If you would like to make this a bit clearer, there could be an `[en|dis]ablePeriodicReply` which has less parameters than `updateReplyMapEntry()`. Prbably none. It only verifies that the reply is indeed periodic and then sets the `delayCycles`.

muellerr commented

2021-09-11 17:10:11 +02:00

Sounds good. So far, I have only needed periodic replies for devices like GPS which just start talking. There should be some form of decoumentation specifying what to do for that use case as well.

muellerr added 1 commit 2021-09-11 17:15:31 +02:00

Merge branch 'development' into mueller/dhb-periodoc-reply-fix a88e97bc09

muellerr added 1 commit 2021-09-11 17:40:11 +02:00

added function to enable periodic reply c9bfc8464a

muellerr commented

2021-09-11 17:40:31 +02:00

I updated the PR

muellerr added 1 commit 2021-09-11 17:42:41 +02:00

added option to disable it as well 11a3c8c21f

muellerr added 1 commit 2021-09-11 17:44:11 +02:00

renamed function 134deb3f43

mohr approved these changes 2021-09-13 10:43:58 +02:00