API Enhancement: Rework set/get length of ACF types

[adriaan-niess]

Using APIs like Avtp_Can_SetAcfMsgLength(...) is error prone since the length is encoded in quadlets. @nayakned We discussed that quite a while ago.

I'd propose to keep only Avtp_AcfCommon_SetAcfMsgLength(...) and remove Avtp_<Type>_SetAcfMsgLength(...) for all ACF types and instead provide a function Avtp_<Type>_SetPayloadLen(...) which uses bytes instead of quadlets and also implicietly sets the correct padding. An implementation could look like this:

void Avtp_Gbb_SetPayloadLen(Avtp_Gbb_t* gbb, uint16_t bytes)
{
    uint8_t pad = (4 - (bytes % 4)) % 4;
    uint16_t quadlets = bytes / 4;
    SET_FIELD(AVTP_GBB_FIELD_ACF_MSG_LENGTH, quadlets);
    SET_FIELD(AVTP_GBB_FIELD_PAD, pad);
}

Analog to removing Avtp_<Type>_SetAcfMsgLength(...), it would then also possible to remove Avtp_<Type>_SetPad(...) because the padding would then implicitely be set by calling Avtp_<Type>_SetPayloadLen(...).

To summarize, I propose to:

• Remove Avtp_<Type>_SetAcfMsgLength(...) for all ACF types (except AcfCommon)
• Remove Avtp_<Type>_GetAcfMsgLength(...) for all ACF types (except AcfCommon)
• Remove Avtp_<Type>_SetPad(...) for all ACF types (except AcfCommon)
• Remove Avtp_<Type>_GetPad(...) for all ACF types (except AcfCommon)
• Add Avtp_<Type>_SetPayloadLen(...) to all ACF types (except AcfCommon)
• Add Avtp_<Type>_GetPayloadLen(...) to all ACF types (except AcfCommon)

This might cause some breaking changes, so the earlier we follow through with this, the better. In case we want to follow through with this, I'd say we should also add a new version tag to the main branch.

@nayakned and @SebastianSchildt what are your thoughts?

[Jignesh-007]

Hello @adriaan-niess, @nayakned and @SebastianSchildt
I would recommend to deprecate the apis Avtp_SetAcfMsgLength(...), AvtpGetAcfMsgLength(...), AvtpSetPad(...), Avtp_GetPad(...) and add new one.
In readme file mark this as deprecate and mention when it will be removed.
After this time frame we can safely remove the api's
this time frame can be 6 Months or 1 Year.

[adriaan-niess]

Thanks @Jignesh-007, deprecating sounds reasonable. This way there's no need to touch existing code. Overall less work which I like.

[nayakned]

• Remove Avtp_<Type>_SetAcfMsgLength(...) for all ACF types (except AcfCommon)
• Remove Avtp_<Type>_GetAcfMsgLength(...) for all ACF types (except AcfCommon)
• Remove Avtp_<Type>_SetPad(...) for all ACF types (except AcfCommon)
• Remove Avtp_<Type>_GetPad(...) for all ACF types (except AcfCommon)
• Add Avtp_<Type>_SetPayloadLen(...) to all ACF types (except AcfCommon)
• Add Avtp_<Type>_GetPayloadLen(...) to all ACF types (except AcfCommon)

I think adding the new APIs could be done right away. I don't think we need to remove the old APIs at all. After all, if someone wants to generate incorrect/malformed packets (e.g. in a test framework) it should be possible. We should have low level APIs which set/get individual fields and slightly abstracted ones which make sure the fields are consistent.

Currently I know of some proprietary projects have these APIs in use. Currently removing these APIs will definitely cause some problems and annoyance.

[SebastianSchildt]

Just ran into length pain yesterday in a project :) I assume we would still need Avtp_<Type>_GetAcfMsgLength(in bytes PLEASE?) to be able to set the length in (N)TSCF header correctly?