You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by GitBox <gi...@apache.org> on 2021/02/15 16:57:37 UTC
[GitHub] [mynewt-mcumgr] greg-leach opened a new pull request #112: Bug #18312 Add support for half-floats to MCUMGR
greg-leach opened a new pull request #112:
URL: https://github.com/apache/mynewt-mcumgr/pull/112
This PR adds half-float support to MCUMGR.
We are using MCUMGR to transfer floating-point data
to our device and have found this can be transferred
as half, single or double precision float depending upon
the value being transferred.
Requested by: Greg Leach <gr...@lairdconnect.com>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-mcumgr] utzig merged pull request #112: Bug #18312 Add support for half-floats to MCUMGR
Posted by GitBox <gi...@apache.org>.
utzig merged pull request #112:
URL: https://github.com/apache/mynewt-mcumgr/pull/112
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-mcumgr] greg-leach commented on a change in pull request #112: Bug #18312 Add support for half-floats to MCUMGR
Posted by GitBox <gi...@apache.org>.
greg-leach commented on a change in pull request #112:
URL: https://github.com/apache/mynewt-mcumgr/pull/112#discussion_r576629119
##########
File path: cborattr/include/cborattr/cborattr.h
##########
@@ -85,6 +86,9 @@ struct cbor_array_t {
struct {
double *store;
} reals;
+ struct{
+ int *store;
+ } halffloats;
Review comment:
The thinking here was to use standard C types, looking further on in the code I can see uint16_t being used. I've updated the halffloat to be a uint16_t type.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-mcumgr] utzig commented on a change in pull request #112: Bug #18312 Add support for half-floats to MCUMGR
Posted by GitBox <gi...@apache.org>.
utzig commented on a change in pull request #112:
URL: https://github.com/apache/mynewt-mcumgr/pull/112#discussion_r576426576
##########
File path: cborattr/src/cborattr.c
##########
@@ -67,6 +67,11 @@ valid_attr_type(CborType ct, CborAttrType at)
}
break;
#if FLOAT_SUPPORT
+ case CborAttrHalfFloatType:
+ if (ct == CborHalfFloatType) {
+ return 1;
+ }
+ break;
Review comment:
Since you are representing a half-float using an integer type, are you sure you need to add it under `FLOAT_SUPPORT`?
##########
File path: cborattr/include/cborattr/cborattr.h
##########
@@ -85,6 +86,9 @@ struct cbor_array_t {
struct {
double *store;
} reals;
+ struct{
+ int *store;
+ } halffloats;
Review comment:
I never used a half-float before, but I'd assume it is 16-bit long (half a float!), so why use `int` instead of `uint16_t`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-mcumgr] greg-leach commented on a change in pull request #112: Bug #18312 Add support for half-floats to MCUMGR
Posted by GitBox <gi...@apache.org>.
greg-leach commented on a change in pull request #112:
URL: https://github.com/apache/mynewt-mcumgr/pull/112#discussion_r576629784
##########
File path: cborattr/src/cborattr.c
##########
@@ -67,6 +67,11 @@ valid_attr_type(CborType ct, CborAttrType at)
}
break;
#if FLOAT_SUPPORT
+ case CborAttrHalfFloatType:
+ if (ct == CborHalfFloatType) {
+ return 1;
+ }
+ break;
Review comment:
Definitely yes. The half float type will only be used when CBOR is compacting float data into a half float type to save on message space.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org