Skip to content

Accept byte array payload for raw CAN and diagnostic message formats #7

Description

@peplin

The raw CAN and diagnostic message payload formats (as JSON) expect a hex string for the payload. Ideally we could use an actual number here, but the JSON spec doesn't include base 16 (hex). Instead, we encode it as a string.

This is problematic for a few reasons, namely that it's inefficient (string manipulation), it's prone to byte ordering issues (between, the host, the VI, and CAN bus ordering).

Really, we shouldn't be trying to represent the payload as a single number, even a uint64_t, at all. It's not a single number, it's an array of bytes. There's no 'byte order' for this because the leftmost byte is always...the leftmost byte. There's no "most significant" byte. We introduced a lot of unnecessary complexity by using uint64_t everywhere in the VI instead of uint8_t[8]. It seemed pretty slick at the time, but it's not helping. This became a bigger issue when we added diagnostic requests, which could have a payload much larger than 8 bytes that would obviously not fit in a single uint64_t.

I think what we should do it change the JSON format to accept a JSON array of numbers (i.e. bytes) for each payload. For backwards compatibility, we may choose to also support hex strings for CAN messages and diagnostic request payloads less than or equal to 6 bytes.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions