config: increase size of ParameterType

[barsnick]

Describe your changes

A size of five proved impractical for real-world use cases. Increase to eight.

ParameterSet is reduced from five to four.

Issue ticket number and link

#72

Checklist before requesting a review

• I have performed a self-review of my code
• I have made corresponding changes to the documentation
• I read the contribution documentation and made sure that my changes meet its requirements

[SebaLukas]

👍

[SiebrenW]

Looking at the schema the ParameterSet should be max=255 and Parameter max=16
Are we fine with this?

[barsnick]

Are we fine with this?

The static memory requirements go totally out of whack if you stick to the schemas' constraints. We need to restrict to some sane (non-embedded) levels, as long as we don't have dynamic memory allocation. (The latter is not feasible for embedded use.)

In both ways, you can change those restrictions. In fact, for our own embedded products, we go down further on most.

BTW, OpenV2G does the same thing (albeit with somewhat different restrictions, and obviously no ISO -20).

The major problem is: There's no sane handling yet if you try to decode a message with more elements than the given buffers can handle.

[SiebrenW]

With larger arrays, you may choose to have dynamic allocation with an arena allocator.
I know for a fact that another, proprietary ISO15118 stack does this for pretty much all of its arrays. A bit overkill and I hear it wasn't easy to read either.
As soon as you're done with the document (after parsing), you can set the allocator to 0 again for reuse.
Then the list can be stored as a linked list.

I would think something like this:

typedef struct exi_arena_alloc_t {
    uint8_t* data;
    size_t data_size;
    size_t byte_pos;
} exi_arena_alloc_t;

typedef struct exi_bitsream_t {
// ...
    exi_arena_alloc_t* alloc;
};
void* exi_arena_alloc_allocate(exi_arena_alloc_t* alloc, size_t size)
{
    if (alloc->data_size < alloc->byte_pos + size) {
        return NULL;
    }
    uint8_t* data = alloc->data + alloc->byte_pos;
    alloc->byte_pos += size;
    return data;
}
void exi_bitstream_init(exi_bitstream_t* stream, uint8_t* data, size_t data_size, size_t data_offset, exi_status_callback status_callback, exi_arena_alloc_t* alloc)
{
//...
    alloc->byte_pos = 0;
}
#define EXI_ALLOCATE(alloc, val) do {val = (typeof(val))exi_arena_alloc_allocate(alloc, sizeof(*val));} while(0)

And the type:

// ====
struct  ParameterTypeNode;
typedef struct ParameterTypeNode {
    iso2_ParameterType data;
    struct ParameterTypeNode *next;
} ParameterTypeNode;
struct iso2_ParameterSetType {
    int16_t ParameterSetID;
    ParameterTypeNode Parameter;
};

and usage:

int decode_ParameterTypeNode(exi_bitstream_t* stream, struct ParameterTypeNode* doc)
{
    EXI_ALLOCATE(stream->alloc, doc->next);
    if (doc->next == NULL) {
        return -1;
    }
    doc->next->next = NULL;
    int res = decode_ParameterType(stream, &doc->next->data);
    return res;
}

Just a suggestion.

[SebaLukas]

I think we can merge this one? @barsnick

[barsnick]

think we can merge this one? @barsnick

Thanks for the ping, we're actively analyzing it now.

[SebaLukas]

I would like to merge the increase of the ParamaterType size quickly. We can then have the discussion about dynamic allecation after that. The too small size causes an encode bug for ISO20, so that the other side can no longer read the exi stream.