Kawai K5000

[christofmuc]

Bank management and sending from kk to synth currently NOT working

[christofmuc]

I need to drop for today, but my main suggestion is that we add a proper make_test_data function here and enable you to run and debug it - this is how I approach harder problems where there might be some bugs lurking. Running this from the Orm itself is much harder to figure out what is going on. I first get the module as a standalone to pass the test before I first try it in the Orm!

[christofmuc]

I have added and pushed the test function, you might want to revisit the adaptation testing guide for this (long time I have looked at it).

When I run the test (it is only one for now), I get this error:

message = [240, 64, None, 32, 0, 10, ...]

    def isSingleProgramDump(message):  # ✅
        # Check minimum length to avoid out-of-bounds errors
        if len(message) < 10:
            return False
    
        # Verify Sysex header for a Kawai K5000 program dump
        return (message[0] == 0xF0  # Start of SysEx
                and message[1] == KawaiSysexID  # Kawai manufacturer ID
>               and 0x00 <= message[2] <= 0x0F  # MIDI channel (0-F for ch 1-16)
                and message[3] == OneBlockDump  # Function ID for single dump
                and message[4] == 0x00  # Reserved
                and message[5] == 0x0A
                and message[-1] == 0xF7)  # End of SysEx
E       TypeError: '<=' not supported between instances of 'int' and 'NoneType'

KawaiK5000.py:146: TypeError

This should be debuggable.

You would need to create a run configuration in Pycharm for pytest, and use these parameters:

[christofmuc]

Don't forget to pull my changes, and I'm off to bed now!

[markusschloesser]

Don't forget to pull my changes, and I'm off to bed now!

will do! me too :-)

[christofmuc]

There is a None channel in the patch produced by your bank loaded:

[markusschloesser]

There is a None channel in the patch produced by your bank loaded:

but i don't understand why? can't def extractPatchesFromAllBankMessages(messages, channel=None) be used with channel? All other adaptations using extractPatchesFromAllBankMessages don't use channel (but it's not that many)

[markusschloesser]

also commited and pushed one single sound sysex to testdata

[markusschloesser]

also you probably guessed it already, but ✅ signals function working, ❌ function not working

[christofmuc]

but i don't understand why? can't def extractPatchesFromAllBankMessages(messages, channel=None) be used with channel? All other adaptations using extractPatchesFromAllBankMessages don't use channel (but it's not that many)

Channel/Sysex ID is confusing admittedly, but the main point here is to not put in None, but a 0. I made a comment on the code in line 367 where you use the channel's default value None. I think this should be 0 (which could be interpreted as sysex ID 1 or Midi channel 1 or whatever=

[markusschloesser]

but i don't understand why? can't def extractPatchesFromAllBankMessages(messages, channel=None) be used with channel? All other adaptations using extractPatchesFromAllBankMessages don't use channel (but it's not that many)

Channel/Sysex ID is confusing admittedly, but the main point here is to not put in None, but a 0. I made a comment on the code in line 367 where you use the channel's default value None. I think this should be 0 (which could be interpreted as sysex ID 1 or Midi channel 1 or whatever=

ok, this fixes that one specific error, now we "just" need to fix the offset calculation 😂
but also i think the first patch is wrongly extracted (wrong name)

00:42:36: info Adaptation: base_ptr calculated: 260
Pointer table has 126 pointers
Processing patch index 9: tone_ptr=0
clean up..................
00:42:36: info Got 1 new or changed patches, saved to database

[markusschloesser]

New code better, but still fails.
Does the flow go from extract bank to issingleprogramdump?

[christofmuc]

New code better, but still fails. Does the flow go from extract bank to issingleprogramdump?

You can see what is going on and what the expectations are by looking at test_adaptations.py, lines 401+

Each patch returned by extractPatchesFromAllBankMessages() has to be accepted by either isSingleProgramDump() or isEditBufferDump(), defined by a flag in the TestData() message returned by make_test_data(). single program dump is default.

So the expectation is that the bank loader produces something that is accepted by isSingleProgramDump().

[christofmuc]

This looks bogus - you split the message into 7 bytes at the front, and everything starting with byte 8 in the end. I'd think the message is one byte longer after this?

[markusschloesser]

This looks bogus - you split the message into 7 bytes at the front, and everything starting with byte 8 in the end. I'd think the message is one byte longer after this?

true, will look at it later in detail.
But do we actually need the function for this adaptation? synth has no edit buffer and extractbank stuff now does its own reconstruction

[markusschloesser]

also you did see my commits from today?

[christofmuc]

Saw them, but didn't check them yet. Probably Saturday!

The convertToProgramDump() is required for the bank functionality, so yes, this needs to be implemented. Should be possible?

[markusschloesser]

Saw them, but didn't check them yet. Probably Saturday!

👍

The convertToProgramDump() is required for the bank functionality, so yes, this needs to be implemented. Should be possible?

why? or where is the sysex header reconstruction supposed to be happening? right now I'm doing formatted_patch = [ 0xF0, KawaiSysexID, 0x00, OneBlockDump, 0x00, 0x0A, 0x00, bank_byte, patch_number, checksum ] + list(current_patch) + [0xF7] at the end of extractPatchesFromAllBankMessages

[markusschloesser]

commit from today fixes this (confirmed successful write by synth)

[christofmuc]

convertToProgramDump() is called when pressing the patch button - as we cannot send to edit buffer when edit buffer is not implemented, the system falls back to patch the last program of the first bank into the program dump and send that to the synth.

So the function ought to inject the correct program position and potentially correct sysex ID/MIDI channel into the patch data before it is sent out.

[christofmuc]

Most adaptations now choose to store either edit buffers or program buffers in the database, and convert them on sending into the appropriate format. So extactPachesFromBank is good to create full program buffers, the convert... function really only alters the program place then.

[markusschloesser]

new code committed, though still not working 🙄

[markusschloesser]

@christofmuc don't check anything for extractPatchesFromBank, got lots of new (uncommitted) code for that. Should be done later this weekend

[christofmuc]

Look what I found: https://github.com/jpcaruana/jsynthlib/tree/master/src/main/java/org/jsynthlib/drivers/kawai/k5000

[markusschloesser]

Check push from last night 😁

[christofmuc]

Perseverance!

KnobKraft would crash if you return an empty bank descriptor array - it tries to calculate the program position for sending the patch into, the last number of the first bank, but if there are no banks, it'll crash right now. Can fix that, but that's for me because it did not detect a K5000 and didn't know the submodel.

[markusschloesser]

Perseverance!

KnobKraft would crash if you return an empty bank descriptor array - it tries to calculate the program position for sending the patch into, the last number of the first bank, but if there are no banks, it'll crash right now. Can fix that, but that's for me because it did not detect a K5000 and didn't know the submodel.

Makes sense, only saw crash if K5000 not switched on (I think)

[markusschloesser]

Is the code understandable?
I still need to look into the additional byte from #433 (comment)

Also did you see my question in #433 (comment)?

[markusschloesser]

New code committed

[christofmuc]

Code looks pretty good, except I didn't dig into the big big function yet. The rest is clear. I added a program test data generator so you get better test coverage, and they all pass, which is good. calculateFingerprint is still missing for proper deduplication.

[markusschloesser]

Code looks pretty good, except I didn't dig into the big big function yet. The rest is clear. I added a program test data generator so you get better test coverage, and they all pass, which is good. calculateFingerprint is still missing for proper deduplication.

did you add to the k5000 branch?? Can only see it in main, which has the old code