Closed randaz81 closed 3 years ago
This is a nice addition @randaz81 👍🏻
In the future, I believe it'd be worth opening up an issue ahead of the PR in order to trigger attention. The PR itself could be enough if we wait a bit before the merge to elicit discussion.
It's not totally clear to me the strategy for the test. If this new module is not validated yet or barely validated, then I would have gone the other way around:
faceExpression_new
faceExpression
After the validation, deprecate (and remove) the old module and announce the new one.
@vvasco keep an eye on this for future integration into assistive-rehab
.
Discussed offline w/ @randaz81:
Complete refactor of
faceExpression
module to make its source code comprehensible.It maintains the same rpc interface of the previous implementation (via vocabs, which should be considered deprecated), plus it adds new string (non -vocab) commands (type 'help' for the list of commands)
The old faceExpression executable has been renamed to
faceExpression_old
. It should be considered deprecated and will be removed in the near future when all functionalities of the implementation are validated by extensive testing.