search

ihedvall / mdflib

Implementation of the ASAM MDF data file.
https://ihedvall.github.io/mdflib/
MIT License
57 stars 25 forks source link

fix: fix Little Endian conversion #58

Closed kenji-myk closed 7 months ago

kenji-myk commented 7 months ago

As I haven't supported Little Endian in https://github.com/ihedvall/mdflib/pull/42, I encountered some issues. (I'm sorry for my missing work at that time.)

I added several modifications to fix the issues:

ihedvall commented 7 months ago

FYI const bool is_big_endian = ChannelDataType() == ChannelDataType::SignedIntegerBe || ChannelDataType() == ChannelDataType::UnsignedIntegerBe;

I found this error after merged when I was addded unit tests: ChannelDataType() should be DataType(). Strange that the compiler didn't generate an error? To be honest, it was the code analyzer that found it.

kenji-myk commented 7 months ago

@ihedvall Hmm, I didn't get any error with GCC. :thinking: Will adding () solve it?

ihedvall commented 7 months ago

FYI I have added CA channel array support to the system. CA support means that each sample record can store an array of values instead of just one value. This makes everything more difficult. I writing unit test that test this new features on the channel.

ihedvall commented 7 months ago

The unit tests fails and it is not an easy fix. Little bit history.

The mdflib used the boost::endian library which fix all conversions back and forward. The endian library did not handle bit offsets > 0 so that is the purpose of the CopyToDataBuffer(), just move the bytes a number of bits.

When I did the dbclib which handle CAN messages. Unlike MDF, it shall support writing with bit offset >0. The DbcHelper functions will handle most evil conversions.

Murmele had requirement of no boost and C++11, why I removed the boost. I also changed from C++20 to C++17.

The question is if I should use the DbcHelper or the Boost::endian library?

kenji-myk commented 7 months ago

Is there any issue with using DbcHelper? :thinking: For me, it's preferable to Boost because it probably has fewer dependencies.

ihedvall commented 7 months ago

I have to remove the copyTo..() function and LittleBuffer/BigBuffer stub. Some disadvantage with the DBCHelper is that it is slower as it always do the "bit-rotation" but the unit tests are simpler.

kenji-myk commented 7 months ago

I see. Removing those functions are okay for me. Regarding speed, I assume it would be acceptable. (If it's slow, then some can who cares speed can contribute to enhance it!)

kenji-myk commented 2 months ago

@ihedvall Hello, I've misunderstood this comment and recognized it now. Sorry for my typo and thank you for your improvements! :pray: