search

robotology / wearables

Code moved to https://github.com/robotology/human-dynamics-estimation
https://github.com/robotology/human-dynamics-estimation
BSD 3-Clause "New" or "Revised" License
19 stars 11 forks source link

Change BufferedPort forceStrict to false #137

Closed lrapetti closed 2 years ago

lrapetti commented 2 years ago

Analogous to https://github.com/robotology/human-dynamics-estimation/pull/279.

In addition in this PR I am addressing https://github.com/robotology/wearables/issues/138

lrapetti commented 2 years ago

Copying here @diegoferigo comment from https://github.com/robotology/human-dynamics-estimation/pull/279 that might be relevant:

Though, considering the wearables architecture, I remember now that we started using forceStrict because in the WearableData message we also include the timestamp. We wanted to be sure that we receive all the messages regardless of the connection status, particularly to ensure that collected datasets would not miss data due to networking problems. In this use case, reordering of message should be done offline, and disabling forceStrict would mean that few samples could be missing. In a real-time setting, however, I don't see problems of disabling forceStrict even with carries different than TCP.

lrapetti commented 2 years ago

For answering @diegoferigo comment above, since the whole infrastructure is mainly headed towards real-time applications, I think we can proceed with disabling forceStrict.

Concerning offline data analysis, we currently have the IWearLogger device for this scope, which, as far as I can see, uses anyway its own receiver timestamp https://github.com/robotology/wearables/blob/ff6bd84a98f3fc01dc44e09af24475c2c72ce1ae/wrappers/IWearLogger/src/IWearLogger.cpp#L263

lrapetti commented 2 years ago

cc @kouroshD @diegoferigo @RiccardoGrieco

lrapetti commented 2 years ago

@RiccardoGrieco @diegoferigo @kouroshD please let me know if we can proceed merging

kouroshD commented 2 years ago

@lrapetti please have a look on this: https://github.com/robotology/walking-teleoperation/pull/87 and https://github.com/robotology/walking-teleoperation/pull/72#discussion_r788965941 The forceStrict set to false as the default option is important to be tested and check if it works as expected, and we do not lose data.

lrapetti commented 2 years ago

Accordingly to the F2F discussion of today, for this use case it is save to use forceStrict set to false

lrapetti commented 2 years ago

I have rebase on top of master, and the PR is ready to be merged

lrapetti commented 2 years ago

Thanks @diegoferigo. Today this modification was further tested. I think we can proceed to merge