aws / aws-iot-fleetwise-edge

Reference Implementation for AWS IoT FleetWise
https://aws.amazon.com/iot-fleetwise/
Apache License 2.0
61 stars 44 forks source link

IEEE 754 support #94

Open apolak opened 4 months ago

apolak commented 4 months ago

Is your feature request related to a problem? Please describe.

CAN databases support IEEE 754 signal values. However, Edge Agent doesn't seem to support this type of signals. In turn, collecting them is not possible. Please see the code below: https://github.com/aws/aws-iot-fleetwise-edge/blob/28f44607f8c0333f408e3a0e7b5628b5846491b9/src/CANDecoder.cpp#L111-L146

In this code, all signals of type float or double are considered to be scaled integers. IEEE 754 numbers are treated as signed 64-bit integers, then converted to double using simple casting.

For example, double value 2.3 is encoded as 0x4002666666666666. The signal is of type double in the signal catalog. However, Edge Agent decodes it as 4612361558371493478 and then casts it to double. As a result, the value stored by AWS IoT FleetWise is 4612361558371493478.0 instead of 2.3.

Describe the solution you'd like

Edge Agent should be able to correctly decode signal values that are 32-bit or 64-bit IEEE 754 numbers. This may require changes to the DecoderManifest in order to be able to identify these types of signals.

Describe alternatives you've considered

  1. Replacing IEEE 754 numbers with scaled integers requires changing message producer/consumer code which may not always be feasible.
  2. Forking Edge Agent increases solution maintenance costs.

Additional context

In DBC files, you can use SIG_VALTYPE_ keyword to specify that the signal value is a 32-bit or 64-bit IEEE 754 number:

signal_extended_value_type_list = 'SIG_VALTYPE_' message_id sig
     nal_name signal_extended_value_type ';' ;

signal_extended_value_type = '0' | '1' | '2' | '3' ; (* 0=signed or 
     unsigned integer, 1=32-bit IEEE-float, 2=64-bit IEEE-double *)

Please see the example below:

import struct

from cantools import database

dbc_data = """
VERSION ""

NS_ :
    NS_DESC_
    CM_
    BA_DEF_
    BA_
    VAL_
    CAT_DEF_
    CAT_
    FILTER
    BA_DEF_DEF_
    EV_DATA_
    ENVVAR_DATA_
    SGTYPE_
    SGTYPE_VAL_
    BA_DEF_SGTYPE_
    BA_SGTYPE_
    SIG_TYPE_REF_
    VAL_TABLE_
    SIG_GROUP_
    SIG_VALTYPE_
    SIGTYPE_VALTYPE_
    BO_TX_BU_
    BA_DEF_REL_
    BA_REL_
    BA_DEF_DEF_REL_
    BU_SG_REL_
    BU_EV_REL_
    BU_BO_REL_
    SG_MUL_VAL_

BS_:

BU_:

BO_ 1 Latitude: 8 Vector__XXX
 SG_ Latitude : 0|64@1+ (1,0) [-90|90] "degrees" Vector__XXX

BO_ 2 Longitude: 8 Vector__XXX
 SG_ Longitude : 0|64@1+ (1,0) [-180|180] "degrees" Vector__XXX

BO_ 3 Speed: 4 Vector__XXX
 SG_ Speed : 0|32@1+ (1,0) [-3.40282347e+38|3.40282347e+38] "km/h" Vector__XXX

SIG_VALTYPE_ 1 Latitude : 2;
SIG_VALTYPE_ 2 Longitude : 2;
SIG_VALTYPE_ 3 Speed : 1;
"""

db = database.load_string(dbc_data, database_format="dbc")

latitude = 1.23
longitude = 4.56
speed = 7.89

# verify signal value is represented as 64-bit IEEE 754 number
assert db.encode_message("Latitude", {"Latitude": latitude}) == struct.pack("d", latitude)
assert db.encode_message("Longitude", {"Longitude": longitude}) == struct.pack("d", longitude)

# verify signal value is represented as 32-bit IEEE 754 number
assert db.encode_message("Speed", {"Speed": speed}) == struct.pack("f", speed)
nenadilic84 commented 4 months ago

Hi Aleksander, thanks for reporting this and providing all the details. We will go over this in more details and get back to you.

If you happen to have the fix at hand, feel free to open a PR.

Thanks once again!

apolak commented 4 months ago

If you happen to have the fix at hand, feel free to open a PR.

@nenadilic84 Thanks for the quick response. I don't have a fix at hand, unfortunately. I think that a general solution would require changes to IoT FleetWise data model - namely, the CanSignal data type - to be able to convey that the signal value is something else than a signed or unsigned integer that you scale and translate, like SIG_VALTYPE_ does. Data type information from the signal catalog is not rich enough as we have three different binary representations of double values and we need to be able to distinguish between them.

hefroy commented 4 months ago

@apolak Thanks for the additional context. It's not clear from the DBC spec what value '3' for SIG_VALTYPE_ means. Perhaps they added a reserved value for future expansion but didn't document it. Do you have any usecase that you need to use that value for? - I note you said "three different binary representations of double values".

apolak commented 4 months ago

@hefroy I think the value 3 is either a reserved value or an error in the specification. The three representations that I had in mind are:

  1. signed or unsigned integer with scale factor (SIG_VALTYPE_ optional) - this is the only representation that works today:

    BO_ 1 Latitude: 4 Vector__XXX
    SG_ Latitude : 0|32@1- (1e-6,0) [-90|90] "degrees" Vector__XXX
    
    SIG_VALTYPE_ 1 Latitude : 0;
  2. 32-bit IEEE 754 number:

    BO_ 1 Latitude: 4 Vector__XXX
    SG_ Latitude : 0|32@1+ (1,0) [-90|90] "degrees" Vector__XXX
    
    SIG_VALTYPE_ 1 Latitude : 1;
  3. 64-bit IEEE 754 number:

    BO_ 1 Latitude: 8 Vector__XXX
    SG_ Latitude : 0|64@1+ (1,0) [-90|90] "degrees" Vector__XXX
    
    SIG_VALTYPE_ 1 Latitude : 2;