Skip to content
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

IEEE 754 support #94

Open
apolak opened this issue Jul 16, 2024 · 4 comments
Open

IEEE 754 support #94

apolak opened this issue Jul 16, 2024 · 4 comments

Comments

@apolak
Copy link

apolak commented Jul 16, 2024

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:

// Start decoding the signal, extract the value before scaling from the Frame.
int64_t rawValue = extractSignalFromFrame( frameData, format.mSignals[i] );
const auto CANsignalType = format.mSignals[i].mSignalType;
switch ( CANsignalType )
{
case ( SignalType::UINT64 ): {
if ( typeid( format.mSignals[i].mFactor ) == typeid( double ) )
{
FWE_LOG_WARN( "Scaling Factor is double for signal ID " +
std::to_string( format.mSignals[i].mSignalID ) + " and type as uint64" );
}
uint64_t physicalRawValue =
static_cast<uint64_t>( rawValue ) * static_cast<uint64_t>( format.mSignals[i].mFactor ) +
static_cast<uint64_t>( format.mSignals[i].mOffset );
auto physicalValue = CANPhysicalValueType( physicalRawValue, CANsignalType );
decodedSignals.emplace_back(
CANDecodedSignal( format.mSignals[i].mSignalID, rawValue, physicalValue, CANsignalType ) );
break;
}
case ( SignalType::INT64 ): {
auto physicalRawValue =
static_cast<int64_t>( rawValue ) * static_cast<int64_t>( format.mSignals[i].mFactor ) +
static_cast<int64_t>( format.mSignals[i].mOffset );
auto physicalValue = CANPhysicalValueType( physicalRawValue, CANsignalType );
decodedSignals.emplace_back(
CANDecodedSignal( format.mSignals[i].mSignalID, rawValue, physicalValue, CANsignalType ) );
break;
}
default: {
auto physicalRawValue =
static_cast<double>( rawValue ) * format.mSignals[i].mFactor + format.mSignals[i].mOffset;
auto physicalValue = CANPhysicalValueType( physicalRawValue, CANsignalType );
decodedSignals.emplace_back(
CANDecodedSignal( format.mSignals[i].mSignalID, rawValue, physicalValue, CANsignalType ) );
}
}

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
Copy link

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
Copy link
Author

apolak commented Jul 17, 2024

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
Copy link
Contributor

hefroy commented Jul 17, 2024

@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
Copy link
Author

apolak commented Jul 17, 2024

@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;
    

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants