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

Some platforms have no uint8_t etc. (one should use uint8_least_t/uint8_fast_t?) #184

Open
TYuD opened this issue Nov 20, 2021 · 7 comments

Comments

@TYuD
Copy link

TYuD commented Nov 20, 2021

Hi!

I see type uint8_t in your code, but some DSPs and MCUs have size of byte bigger then 8 bit. There are examples of stdint.h file (C/C++, no type uint8_t ):

// -------------------------------------- ADSP TS-201 compiler:

typedef unsigned int uint32_t;

typedef uint32_t uint_least8_t;
typedef uint32_t uint_least16_t;
typedef uint32_t uint_least32_t;

typedef uint32_t uint_fast8_t;
typedef uint32_t uint_fast16_t;
typedef uint32_t uint_fast32_t;

// -------------------------------------- TMS320C2804 compiler:

typedef unsigned int uint16_t;
typedef unsigned long uint32_t;

typedef uint16_t uint_least8_t;
typedef uint16_t uint_least16_t;
typedef uint32_t uint_least32_t;

typedef uint16_t uint_fast8_t;
typedef uint16_t uint_fast16_t;
typedef uint32_t uint_fast32_t;

I think you should use uint8_least_t/uint8_fast_t instead of uint8_t etc.

I'm wrong?

@TYuD TYuD changed the title Some platforms have no uint8_t etc. (one should use uint8_least_t/uint8_least_t?) Some platforms have no uint8_t etc. (one should use uint8_least_t/uint8_fast_t?) Nov 20, 2021
@pavel-kirienko
Copy link
Member

I don't think you can easily replace uint8 as it would likely break transfer payload management, which assumes that the buffer memory is byte-addressable. If you experimented with it and reported back your findings, I think that would be useful to many users, as similar issues were raised in the past (e.g., #55).

@TYuD
Copy link
Author

TYuD commented Nov 20, 2021

Sorry. I liked your startup. It is much demanded. But I can't use it.

I use bitfields in structures and bitshifting to pack data:

//----------------------------------------------------
typedef struct
{
uint_fast16_t TypeFormat : 16;
uint_fast8_t SystemFlags: 8 ;
uint_fast8_t QueryID : 8 ;
uint_fast32_t Reserve : 32;
}
TPressureSensorQuery;
//----------------------------------------------------
typedef struct
{
uint_fast16_t TypeFormat : 16;

uint_fast8_t Error : 1 ;
uint_fast8_t Ready : 1 ;
uint_fast8_t SystemFlags: 6 ;
uint_fast8_t UserFlags : 8 ;

uint_fast32_t Pressure : 32;

uint_fast32_t Time : 32;
uint_fast32_t RareAuxData: 32;
uint_fast8_t RareAuxID : 8 ;
uint_fast32_t Reserve : 24;
uint_fast32_t AuxData1 : 32;
uint_fast32_t AuxData2 : 32;
}
TPressureSensorAnswer;

@TYuD
Copy link
Author

TYuD commented Nov 20, 2021

Can you highlight typical sections of code where the discussed problem brings difficulties? Together we could think about how to solve them. It would be easier for me if these sections were in C/C++ language.

@pavel-kirienko
Copy link
Member

I would recommend you to approach it this way instead: modify the codebase to use uint_least8_t in place of uint8_t and see if that causes breakage. You will need to modify the test suite to run on your platform though, which may prove tedious, so you should start with the most basic tests that validate the fundamentals, like https://github.com/UAVCAN/libcanard/blob/master/tests/test_private_rx.cpp.

@TYuD
Copy link
Author

TYuD commented Nov 20, 2021

It is difficult for me to understand much amount of your code.

Types like uint8_fast_t I use for a long time. There are no problems with them as variables or structure fields.

Problems can be with pointers. They can not offset to 8 bits (uint8_t) in some platforms. In such a case one must use bit operations to pack/unpack data instead of pointer offset. I think you do just so when pack uintX_t (X<8, DSDL) into a message. In same a way data may be packet into char array, when size of char is bigger 8 bit. Char size may be parameterized in your code. Its value contain CHAR_BIT macro in file limits.h. Also this value can be pointed as a option of DSDL translators like little/big endean. It may be considered like a MTU value in transport layer.

@TYuD
Copy link
Author

TYuD commented Jan 9, 2022

Hi, Pavel!

I have slightly modified the nunavutCopyBits function (I think this is the core function of serilization). Now it looks the nunavutCopyBits can work on platforms with different byte lengths. Even a little faster (without divisions). In the same way, one can modify the other functions of the library.

It is here:
https://github.com/TYuD/hello_world/blob/master/serialization_2.h

Here are some small checks on the PC and on the TigerSHARC processor:

https://github.com/TYuD/hello_world/blob/master/main.cpp

@pavel-kirienko
Copy link
Member

I opened a new issue over at Nunavut to track this: OpenCyphal/nunavut#233

Even a little faster (without divisions)

With any half-decent compiler, replacing divisions with shifts adds nothing but obscurity.

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

No branches or pull requests

2 participants