-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Can battery #63
Can battery #63
Conversation
Hi @triccyx Remember that we need to handle the new version of the suite. cc @marcoaccame |
Uhm, what does this mean? |
It means this: https://github.com/robotology/icub-firmware-shared/pull/58/files#diff-1e7de1ae2d059d21e1dd75d5812d5a34b0222cef273b7c3a2af62eb747f9d20a. Get in touch with @marcoaccame about how to proceed here. |
@marcoaccame fixed:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @triccyx:
- remember to increase
VERSION
in https://github.com/robotology/icub-firmware-shared/blob/devel/CMakeLists.txt#L9 - in
eOas_battery_config_t::period
the comment is misleading because it mentionsstrain2
andstrain
boards. You can just write something as: // if 0 -> DONT TX, else TX - all the rest is fine.
Ok done @marcoaccame |
@marcoaccame you should review as @pattacini does. It is easier to find the problems in this way. (I mean not using comments) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @triccyx: I forgot two small changes:
- advance protocol version of management endpoint to be 2.22 (see https://github.com/robotology/icub-firmware-shared/blob/devel/eth/embobj/plus/comm-v2/protocol/api/EoProtocolMN.h#L61)
- advance protocol version of analog sensor endpoint to be 1.7 (see https://github.com/robotology/icub-firmware-shared/blob/devel/eth/embobj/plus/comm-v2/protocol/api/EoProtocolAS.h#L60)
@marcoaccame note that |
done |
Yes it is. however, you still need to update protocol version of management |
done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can I merge the PR @marcoaccame?
Added to FW shared the Can battery board.