Removing Struct Parameter of encode/decode function?

[dillasyx]

What about removing the struct parameter of the encode and decode functions, as it should always be the same for the signal. Or is there anything I miss, why the possibilty should be given?

bool decode_can_0x015_SUFUB_CURRENT_MAIN_A(can_0x015_SUFUB_STATUS_t *record, double *out); bool encode_can_0x015_SUFUB_CURRENT_MAIN_A(can_0x015_SUFUB_STATUS_t *record, double in);

to

bool decode_can_0x015_SUFUB_CURRENT_MAIN_A(double *out); bool encode_can_0x015_SUFUB_CURRENT_MAIN_A(double in) ;

[howerj]

Hi!

If you check the code base, I updated the project last night so that it code generation is slightly different. A single, very large, structure is defined, which contains all of the CAN messages, which in turn contain the signals. A pointer to this large structure is now passed to each of the encode/decode functions instead of a pointer to a specific record, this makes the generated code less flexible but easier to use. Not only this, no variables of static storage duration are present in the generated code any more, it is up to the programmer to allocate this variable wherever they want.

I am in the process of changing the generated code so status flags are also present and can be controlled, which is always useful when dealing with CAN messages. It will need thinking about.

Thanks,
Richard

[dillasyx]

As an improvement, how about using uint8_t for function like pack, unpack, decode, encode as the return value should never be that big, that an int is needed.

Btw you could use attribute((packed)) in front of the struct name. It's used to suppress data alignment.

Just some thoughts.

[howerj]

Thanks for the suggestions,

I don't think using 'uint8_t' would be an improvement, the common C idiom is to return a negative on failure and zero or positive on success, in the case of pack/unpack/etcetera functions - you shouldn't care why it has failed. To me, when I see a 'uint8_t' it means there is a reason why, why specifically that width of value has been chosen. Some people chose to return a 'uint16_t' or a 'uint32_t' instead, and in code bases where using fixed width types is enforced I have often see this inconsistency regardless of how many possible values are returned. I think it is best to just return an 'int' and not think about (also, the compiler is slightly more likely to generate shorter code for 'int' than 'uint8_t').

A better way then using "__attribute__((packed))", a non-portable GCC specific construct, would be to define macros PREPACK and POSTPACK that the user can define:

   #ifndef PREPACK
   #define PREPACK
   #endif       
   #ifndef POSTPACK
   #define POSTPACK
   #endif
   typedef PREPACK struct {
      int x, y, z;
   } POSTPACK name_of_struct_t;

You could then allow the user the option to pack the data if they wish, which tends to lead to a slower program - but not always, given their compilers extension for doing so. I think I will implement this.

I don't want to discourage suggestions, which is why I have put a detailed reasoning for each of my points. Thank you for your time and feedback, and feel free to disagree.

[howerj]

I have added the PREPACK and POSTPACK mechanism.