peplin
commented
10 years ago I just pushed an update to the docs in a branch with this change, but I want to update the vi-firmware and openxc-python projects to make sure it isn't too awkward before I commit to it.
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.