Missing _isUsed check for some optional fields

[bklop]

First of all, what a fantastic project, thanks for releasing it!

I've been running cbEXIgen and comparing the output to OpenV2G.

I noticed that for some fields, the cbEXIgen generated code does not check for the _isUsed flag before encoding a field.

For example: in the DIN SPEC 70121 encoder encode_din_PMaxScheduleEntryType, field PMaxScheduleEntryType->TimeInterval is accessed like this:

            if (PMaxScheduleEntryType->RelativeTimeInterval_isUsed == 1u)
            {
                // ... encoding rules, removed since it's not relevant for this bug report...
            }
            else
            {
                error = exi_basetypes_encoder_nbit_uint(stream, 2, 1);
                if (error == EXI_ERROR__NO_ERROR)
                {
                    // Abstract element or type: START (IntervalType); next=13
                    error = encode_din_IntervalType(stream, &PMaxScheduleEntryType->TimeInterval);
                    if (error == EXI_ERROR__NO_ERROR)
                    {
                        grammar_id = 13;
                    }
                }
            }

I would expect the else case to check PMaxScheduleEntryType->TimeInterval_isUsed, like OpenV2G does: https://github.com/Martin-P/OpenV2G/blob/671f422a73cb9055b0469ea00ab14e3a46992ae4/src/din/dinEXIDatatypesEncoder.c#L4278C3-L4278C3

[barsnick]

In principle, you are correct.

The problem is not the missing check of _isUsed, as these else blocks are always filled with the abstract type's encoder block. The pure abstract types are not meant to ever be used. (Encoding them seems possible, but is not legally defined.)[^1]

In the currently created C code, you erroneously reach this block by not properly defining the (or, in other cases, any of the) derived element(s), possibly by having forgotten to initialize it/them.

Instead of this block implementing an encoder, it should probably throw an error and refuse to encode the packet.

Strictly speaking, cbExiGen is creating code which works correctly, but may be considered too tolerant regarding false use of the interface. We or the community may be able to improve this behavior. (Internally, these types are already marked as "abstract", the generated code would need to be explicitly replaced with an error block.)

OpenV2G behaves just as incorrectly by creating an encoding when TimeInterval_isUsed is set to 1.

[^1]: TimeInterval is of an abstract type, RelativeTimeInterval is a proper, derived element. EXI requires both the realization and the abstract parent to be used for counting the event codes. TimeInterval does not represent any properly encodable type though. See also e.g. EVSEStatus, which is derived to DC_EVSEStatus and AC_EVSEStatus.

[bklop]

I guess the abstract types could be removed from the data types altogether? Anyway, thanks for the explanation, I will close this issue :).