[adaptations\YamahaRefaceDX.py]: dataBlockFromMessage not quite to spec?

[milnak]

Reface MIDI data spec says: The Check-sum is the value that results in a value of 0 for the lower 7 bits when the Model ID, Start Address, Data and Check sum itself are added.

Current implementation is (which does work, so consider this a nit):

def dataBlockFromMessage(message):
    if isOwnSysex(message):
        data_len = message[5] << 7 | message[6]
        if len(message) == data_len + 9:
            data_block = message[8:-2]
            check_sum = -0x05
            for d in data_block:
                check_sum -= d
            if (check_sum & 0x7f) == message[-2]:
                # Check sum passed
                return data_block
            else:
                raise Exception("Checksum error in reface DX data")
    raise Exception("Got corrupt data block in refaceDX data")

but it seems more correct to do something like:

def dataBlockFromMessage(message):
    # Byte Count shows the size of data in blocks from Model ID onward
    # (up to but not including the checksum).
    byte_count = message[5] << 7 | message[6]
    if len(message) == byte_count + 9:
        # The Check-sum is the value that results in a value of 0 for the
        # lower 7 bits when the Model ID, Start Address, Data and Check sum itself are added.
        checksum_block = message[0x07:-1]
        if ((sum(checksum_block) & 0x7f) == 0):
            # return "Data" block
            return message[0x0B:-2]
    raise Exception("Checksum error")

[christofmuc]

I looked at this again to understand what happened, but it is more mysterious than I thought.

The original implementation of my refaceDX adaptation was in C++, it is here:

https://github.com/christofmuc/MidiKraft-yamaha-refacedx/blob/2f650e3ecc53dfd03e52a5be3a336a73784358ee/RefaceDX.cpp#L213

That code is even older and comes from a never published predecessor to Knobkraft. And strangely, in the same file there is another implementation which is more inline with your version:

                int sum = 0;
        std::for_each(bulkDump.begin() + 6, bulkDump.end(), [&](uint8 byte) { sum -= byte; });
        bulkDump.push_back(sum & 0x7f);

Note that it does subtract instead of add - doesn't make a difference I guess when the result should be 0 in the end after dropping additional bits. I think we can take your version as it certainly is cleaner to read!

[christofmuc]

Fixed in 2.4.0