OpenCyphal / nunavut

Generate code from DSDL using PyDSDL and Jinja2
https://nunavut.readthedocs.io/
Other
39 stars 24 forks source link

[C] Nunavut uavan.register.value doesn't generated defines for tags #165

Closed PetervdPerk-NXP closed 3 years ago

PetervdPerk-NXP commented 4 years ago

The UAVCAN register messages uses tags to diffferentiate between data types which are in the register. However to lookup the register definition, you've to check manually the comments in https://github.com/UAVCAN/public_regulated_data_types/blob/master/uavcan/register/Value.1.0.uavcan.

It would be nice that the Value_1_0.h header file would include the defines for these register index types, for the sake portability and ease of use.

pavel-kirienko commented 4 years ago

If you scroll to the bottom of Value_1_0.h, you will see union tag manipulation functions there:

/// Mark option "empty" active without initializing it. Does nothing if @param obj is NULL.
static inline void uavcan_register_Value_1_0_select_empty_(uavcan_register_Value_1_0* const obj)
{
    if (obj != NULL)
    {
        obj->_tag_ = 0;
    }
}

/// Check if option "empty" is active. Returns false if @param obj is NULL.
static inline bool uavcan_register_Value_1_0_is_empty_(const uavcan_register_Value_1_0* const obj)
{
    return ((obj != NULL) && (obj->_tag_ == 0));
}

/// Mark option "string" active without initializing it. Does nothing if @param obj is NULL.
static inline void uavcan_register_Value_1_0_select_string_(uavcan_register_Value_1_0* const obj)
{
    if (obj != NULL)
    {
        obj->_tag_ = 1;
    }
}

/// Check if option "string" is active. Returns false if @param obj is NULL.
static inline bool uavcan_register_Value_1_0_is_string_(const uavcan_register_Value_1_0* const obj)
{
    return ((obj != NULL) && (obj->_tag_ == 1));
}

etc...

The reason we do it this way is that it is (marginally) more type-safe and because this is what Scott prefers: https://github.com/UAVCAN/nunavut/pull/115#issuecomment-695126547 (see the linked comment and the few preceding ones).

We can trivially generate an enumeration for the tag values if this is considered valuable but there is some risk that it might incentivize somewhat less safe coding practices. Do we want this?

thirtytwobits commented 3 years ago

I'm inclined to close this unless @PetervdPerk-NXP disagrees?

PetervdPerk-NXP commented 3 years ago

Yes you can close this issue, based on Pavel's explanation it makes sense to implement it this way.