bsp2 / libanalogrytm

Portable Sysex Library for the Elektron Analog Rytm Drum Computer
MIT License
54 stars 9 forks source link

Any plans to test it for FW 1.70 #4

Open alisomay opened 9 months ago

alisomay commented 9 months ago

If you'd share what to look for during testing and how you approach reverse engineering briefly I'm up for testing and contributing also ✨

bsp2 commented 9 months ago

I'm already on it (FW1.70 Kit+Sound editing is working, pattern format analysis pending). Update is coming soon.

alisomay commented 9 months ago

Wow that is amazing that you're still maintaining this! 👏 Please let me know if there is anything I can help with. Currently I'm making a Rust port using the bindings from this library, if this is still maintained I'm going to update it to point to this one. 🥳

https://github.com/alisomay/rytm-sys

Here are the raw bindings, I'm currently building the safe wrapper around these to make it usable in Rust.

alisomay commented 9 months ago

Here is also a debug output from me with the current version of the library with a test syx file if it helps.

syx_and_debug.zip

alisomay commented 9 months ago

Wow that is amazing that you're still maintaining this! 👏 Please let me know if there is anything I can help with. Currently I'm making a Rust port using the bindings from this library, if this is still maintained I'm going to update it to point to this one. 🥳

https://github.com/alisomay/rytm-sys

Here are the raw bindings, I'm currently building the safe wrapper around these to make it usable in Rust.

Updated rytm-sys to track this repo instead.

bsp2 commented 9 months ago

Rust, nice :-) Always wanted to learn that one but never found the time.

I'm still busy with analyzing the new pattern format. It has changed quite a bit, for example (and as a "heads up"): the trigs are now stored as bitstreams with 14 bits per trig. The ar_pattern_track_t.trigs field, which used to be a u16[64] array, is now a ((14*64)/8)=112 bytes buffer:

sU8 trig_bits[((14*64)/8)]; /* @0x0004..0x0073.  See AR_TRIG_xxx flags. 14 bits per step ((14*64)/8)==112 bytes */

i.e. you'll need functions that can read / write these bits within a byte buffer. I just implemented this in my C++ framework and added prt and pwt (read/write trig) debug commands to my editor for verification and further analysis.

alisomay commented 9 months ago

Thanks for letting me know! I think that wouldn't be a big issue maybe I can experiment and write them before hand.

Just out of curiosity, You had documented:

/* (note) big endian */
#define AR_TRIG_SWING     (0x0010u)  /* <void> swing                       */
#define AR_TRIG_NOTE_ON   (0x0001u)  /* note trig ("enable")               */
#define AR_TRIG_SLIDE     (0x0020u)  /* <void> slide                       */
#define AR_TRIG_UNKNOWN1  (0x0002u)  /* ?                                  */
#define AR_TRIG_UNKNOWN6  (0x0040u)  /* ?                                  */
#define AR_TRIG_MUTE      (0x0004u)  /* <void> mute                        */
#define AR_TRIG_ACCENT    (0x0008u)  /* <void> accent                      */
#define AR_TRIG_SYN_EN    (0x0800u)  /* enable synswitch p-lock            */
#define AR_TRIG_SYN_SW    (0x0080u)  /* synswitch bit (set=on, unset=off)  */
#define AR_TRIG_SMP_EN    (0x1000u)  /* enable smpswitch p-lock            */
#define AR_TRIG_SMP_SW    (0x0100u)  /* smpswitch bit (set=on, unset=off)  */
#define AR_TRIG_ENVF_EN   (0x2000u)  /* enable filter env p-lock           */
#define AR_TRIG_ENVF_SW   (0x0200u)  /* filter env bit (set=on, unset=off) */
#define AR_TRIG_LFO_EN    (0x4000u)  /* enable LFO p-lock                  */
#define AR_TRIG_LFO_SW    (0x0400u)  /* LFO bit (set=on, unset=off)        */
#define AR_TRIG_RETRIG    (0x8000u)  /* <void> retrig                      */
#define AR_TRIG_TRIGLESS  (0x7801u)  /* trigless trig (mask=0x7801)        */

Total 17 combinations for a 16 bit big endian flag. Did this list expand or did you find the meaning of unknown combinations?

How do you approach the analysis generally?

Do you just:

?

bsp2 commented 9 months ago

it's basically just that. the alternative would be to dump/disassemble/reverse-engineer the firmware but that would require more time and effort (and also money to buy the required JTAG emulator).

the trig flags are most likely just single bits per option, the combined values were just observations (which may not be valid anymore, haven't checked all of them, yet).

alisomay commented 9 months ago

I'm now preparing an environment to analyze the new style flags according to the 14 bit per trig definition. I'll share my findings also maybe we compare them later on ✨

alisomay commented 9 months ago

Analyzing:

Empty project load:

TRIG_0: 0000_0000_0000_0000 FLAGS: []
TRIG_1: 0000_0000_0001_0000 FLAGS: ["SWING"]

Adding TRIG_0 to the sequencer:

TRIG_0: 0000_0011_1000_0001 FLAGS: ["ENVF_SW", "SMP_SW", "SYN_SW", "NOTE_ON"]
TRIG_1: 0000_0000_0001_0000 FLAGS: ["SWING"]

Removing TRIG_0 from the sequencer:

TRIG_0: 0000_0011_1000_0000 FLAGS: ["SMP_SW", "ENVF_SW", "SYN_SW"]
TRIG_1: 0000_0000_0001_0000 FLAGS: ["SWING"]

Adding TRIG_0 again then muting it.

TRIG_0: 0000_0011_1000_0101 FLAGS: ["NOTE_ON", "MUTE", "ENVF_SW", "SMP_SW", "SYN_SW"]
TRIG_1: 0000_0000_0001_0000 FLAGS: ["SWING"]

Adding an accent to the same trig.

TRIG_0: 0000_0011_1000_1101 FLAGS: ["ENVF_SW", "NOTE_ON", "SYN_SW", "ACCENT", "MUTE", "SMP_SW"]
TRIG_1: 0000_0000_0001_0000 FLAGS: ["SWING"]

Adding swing and slide to the same trig.

TRIG_0: 0000_0011_1011_1101 FLAGS: ["ACCENT", "SLIDE", "SMP_SW", "SYN_SW", "ENVF_SW", "NOTE_ON", "SWING", "MUTE"]
TRIG_1: 0000_0000_0001_0000 FLAGS: ["SWING"]

Removing mute, accent, swing and slide from the trig.

TRIG_0: 0000_0011_1000_0001 FLAGS: ["NOTE_ON", "SMP_SW", "SYN_SW", "ENVF_SW"]
TRIG_1: 0000_0000_0001_0000 FLAGS: ["SWING"]

Enabling retrig for the same trig. This one is interesting, I guess the retrig flag is changed because now UNKNOWN1 mask points to it

TRIG_0: 0000_0011_1000_0011 FLAGS: ["SYN_SW", "ENVF_SW", "SMP_SW", "NOTE_ON", "UNKNOWN1"]
TRIG_1: 0000_0011_1001_0000 FLAGS: ["SMP_SW", "ENVF_SW", "SWING", "SYN_SW"]

I don't know how to make a trigless trig so I pass that for now :)

Another interesting find is after enabling parameter locks for syn,smp,env and lfo for the trig. The enabling of lfo points to UNKNOWN6 now.

TRIG_0: 0011_1011_1100_0001 FLAGS: ["SYN_SW", "SYN_EN", "ENVF_SW", "SMP_EN", "SMP_SW", "ENVF_EN", "UNKNOWN6", "NOTE_ON"]
TRIG_1: 0000_0011_1001_0000 FLAGS: ["SWING", "ENVF_SW", "SMP_SW", "SYN_SW"]

To be continued..

Summary: Most of the existing flags work! 🥳 We just need to do a little rework on masks and maybe discover some more flags. I'll continue the discovery as soon as I find time again (soon probably.)

alisomay commented 9 months ago

If we summarize the findings with some further comparisons I'm quite confident about this mapping:

Bit order: 13 -> ENV_EN 12 -> SMP_EN 11 -> SYN_EN 10 -> LFO_EN 09 -> ENV_SW 08 -> SMP_SW 07 -> SYN_SW 06 -> LFO_SW (previously UNKNOWN6) 05 -> SWING 04 -> SLIDE 03 -> ACCENT 02 -> MUTE 01 -> RETRIG (previously UNKNOWN1) 00 -> NOTE_ON

alisomay commented 9 months ago

And here is a suggestion if it will be useful to you ✨

/* Bit mapping for sequencer triggers, ordered from bit 0 to bit 13 */
#define AR_TRIG_NOTE_ON   (0x0001u)  /* Bit 00: Note trigger enable (NOTE_ON) */
#define AR_TRIG_RETRIG    (0x0002u)  /* Bit 01: Retrigger activation  */
#define AR_TRIG_MUTE      (0x0004u)  /* Bit 02: Mute feature activation */
#define AR_TRIG_ACCENT    (0x0008u)  /* Bit 03: Accent feature activation */
#define AR_TRIG_SLIDE     (0x0010u)  /* Bit 04: Slide feature activation */
#define AR_TRIG_SWING     (0x0020u)  /* Bit 05: Swing feature activation */
#define AR_TRIG_LFO_SW    (0x0040u)  /* Bit 06: LFO switch (set=on, unset=off) */
#define AR_TRIG_SYN_SW    (0x0080u)  /* Bit 07: SYN switch (set=on, unset=off) */
#define AR_TRIG_SMP_SW    (0x0100u)  /* Bit 08: SMP switch (set=on, unset=off) */
#define AR_TRIG_ENV_SW    (0x0200u)  /* Bit 09: ENV switch (set=on, unset=off) */
#define AR_TRIG_LFO_EN    (0x0400u)  /* Bit 10: Enable LFO modulation p-lock */
#define AR_TRIG_SYN_EN    (0x0800u)  /* Bit 11: Enable SYN modulation p-lock */
#define AR_TRIG_SMP_EN    (0x1000u)  /* Bit 12: Enable SMP modulation p-lock */
#define AR_TRIG_ENV_EN    (0x2000u)  /* Bit 13: Enable ENV modulation p-lock */

/* The upper two bits of the 16-bit value are always 0, ensuring the value is within 16-bit range */
#define AR_TRIG_TRIGLESS  (0x7C01u)  /* Mask for trigless trig (combination of bits 13 to 01) */

I've assumed the AR_TRIG_TRIGLESS that one is not tested.

bsp2 commented 9 months ago

keep in mind that these are tightly packed 14bit values, i.e. there are no "upper two bits of the 16-bit value".

trigless trigs are set via [FUNC+TRIG] (or by p-locking the SYN+SMP+ENV+LFO switches and setting them all to 0), i.e. there's no extra bit for this.

I've updated the flags as following so far:

#define AR_TRIG_NOTE_ON    (1u <<  0)  /* (0x0001u)        note trig [TRIG]         */
#define AR_TRIG_RETRIG     (1u <<  1)  /* (0x0002u) <ali>  retrig    [FUNC+Retrig+] */
#define AR_TRIG_MUTE       (1u <<  2)  /* (0x0004u) <void> mute      [FUNC+A/E]     */
#define AR_TRIG_ACCENT     (1u <<  3)  /* (0x0008u) <void> accent    [FUNC+B/F]     */
#define AR_TRIG_SWING      (1u <<  4)  /* (0x0010u) <void> swing     [FUNC+C/G]     */
#define AR_TRIG_SLIDE      (1u <<  5)  /* (0x0020u) <void> slide     [FUNC+D/H]     */
#define AR_TRIG_LFO_PL_SW  (1u <<  6)  /* (0x0040u)        p-locked LFO state       */
#define AR_TRIG_SYN_PL_SW  (1u <<  7)  /* (0x0080u)        p-locked SYN state       */
#define AR_TRIG_SMP_PL_SW  (1u <<  8)  /* (0x0100u)        p-locked SMP state       */
#define AR_TRIG_ENV_PL_SW  (1u <<  9)  /* (0x0200u)        p-locked ENV state       */
#define AR_TRIG_LFO_PL_EN  (1u << 10)  /* (0x0400u)        enable LFO p-lock        */
#define AR_TRIG_SYN_PL_EN  (1u << 11)  /* (0x0800u)        enable SYN p-lock        */
#define AR_TRIG_SMP_PL_EN  (1u << 12)  /* (0x1000u)        enable SMP p-lock        */
#define AR_TRIG_ENV_PL_EN  (1u << 13)  /* (0x2000u)        enable ENV p-lock        */
alisomay commented 9 months ago

I think that aligns with my analysis also! Great 🥳

"keep in mind that these are tightly packed 14bit values, i.e. there are no "upper two bits of the 16-bit value"." Maybe my expression is misleading. Yes they're tightly packed but since there is no u14 primitive it makes sense to pad them with two 0 MSBs and parse them to u16 values when analyzing. You basically ignore the upper two bits and still have the data semantically.

So we're on the same page.

bsp2 commented 9 months ago

unfortunately the weekend here is almost over and I have to work next week so I'll probably won't have much time to further analyze this until next weekend. There's plenty of work left (let alone updating the application, e.g. the pattern import methods).

On the upside, the kit+sound update works nicely so far and, in my editor, old FW1.50/1.61 sounds are automatically converted to the new format. Also added a lot of tables to display values as names instead of just 0..127 (e.g. waveforms, speeds, ..).

Last but not least: It's a very nice OS update, have already made some nice patches with the new machines :-) (although most of the time was spent on the format analysis, which, as you may have noticed, is not exactly fun)

alisomay commented 9 months ago

Some more findings, if it also helps.

A track structure:

rytm-track

This one repeats 13 times as you've analyzed before. I've picked the orange color for unknown parts.

red -> Magic header cyan -> 14bit packed trig flags until the colorful part -> Same structure as before colorful part :

We now have 7 bytes here and one orange section.

The old analysis was

   sU8     trig_note;                   /* @0x0244           <void> trigNote                          */
   sU8     trig_velocity;               /* @0x0245           <void> trigVelocity                      */
   sU8     trig_note_length;            /* @0x0246           <void> trigLength                        */
   s_u16_t trig_flags;                  /* @0x0247           <void> trigFlags (BIG ENDIAN!)           */
   sU8     __unknown1;                  /* @0x0249           <void> unknown                           */
   sU8     num_steps;                   /* @0x024A           Number of steps (1..64)                  */
   sU8     quantize_amount;             /* @0x024B           <void> quantizeAmount                    */

which was 8 bytes. I think num_steps is the one after the orange section. But I didn't start playing around to figure the orange section yet.

gray part -> sound locks unknown end orange section -> This part has 10 bytes which are waiting to be analyzed.

I'll try to find what the orange parts are and test the others, if you find something out and have the time I'd be happy if you share it.

Here is the current grammar file if you have compatible software to use it such as Synalyze It! or Hexinator.

rytm_pattern_170.grammar.zip

alisomay commented 9 months ago

unfortunately the weekend here is almost over and I have to work next week so I'll probably won't have much time to further analyze this until next weekend. There's plenty of work left (let alone updating the application, e.g. the pattern import methods).

On the upside, the kit+sound update works nicely so far and, in my editor, old FW1.50/1.61 sounds are automatically converted to the new format. Also added a lot of tables to display values as names instead of just 0..127 (e.g. waveforms, speeds, ..).

Last but not least: It's a very nice OS update, have already made some nice patches with the new machines :-) (although most of the time was spent on the format analysis, which, as you may have noticed, is not exactly fun)

Yeah but I'm really happy that you're doing it. I loved the update also, I'll try to contribute as much as possible. It might give you some speed in your analysis. I'll continue the thread whenever I find more information ✨ It is not always fun but I think it will be rewarding.

alisomay commented 9 months ago

And here are the orange part in the pattern footer just to be complete. Which I'll try to find out.

plock_seq_and_footer
bsp2 commented 9 months ago

I also have these notes and logs but in the end you have to identify and test each and every parameter. Posting every single step in the process is probably not so helpful (too much text to wade through). Better post header file updates that can easily be diff'ed.

There are also some (relatively large) blind spots left in the kit format, things like

   sU8 __unknown_arr5[54];     /* @0x09D9..0x0A0E */
   sU8 __unknown_arr6[35];     /* @0x0A0F..0x0A31 */

no idea what's stored in there.

alisomay commented 9 months ago

Got you there 👍 I'll compile some findings, test them and make aggregate posts. Currently I'm more focused to the pattern format since that one is the most important one for me but when that is done I'd gladly take a look to the kit format also.

bsp2 commented 9 months ago

Are you writing some kind of utility (pattern generator or editor?)

alisomay commented 9 months ago

Yep, I have an aggregate set where I use rytm also. I have a single board computer which I use my custom applications for event processing, routing other messages, keeping states, generating randomness, gathering information from devices etc. I want to know what is going on with rytm in my application and ocasionally generate and write patterns triggered by my controllers.

Especially getting information about the length of a pattern might open up the possibility to generate randomness on euclidian seq parameters on pattern loops. That is one idea which might be quite powerful imo.

bsp2 commented 9 months ago

My own main interest is in the sound / kit data. There is a pattern import feature in my sequencer, though. Useful for starting drafts with the Rytm, then importing + refining them in the sequencer (which can already do all this randomness / event modulation / scripting / euclidean pattern generation).

Exporting sequences to the Rytm (including ones that span multiple patterns) has been on my todo list for quite some time but there always were more importing things to do (I am also developing a sampler, an FM synth, and a modular synth that can emit native code).

But yeah, we'll eventually get there. Hopefully before the SysEx format is changed again. :^) (this is now the fifth time..)

alisomay commented 9 months ago

Sounds amazing! Yes the dreaded magic header is 00 00 00 05 😆 I hope also.

bsp2 commented 9 months ago

The fact that the version number is 32 bit is a tiny bit worrying. Plenty of room for another 4294967291 updates ^_^

bsp2 commented 9 months ago

I've just pushed the current work-in-progress pattern.h update.

Parameters marked with ?@ are guesses which need to be tested and confirmed, or updated.

alisomay commented 9 months ago

Great! 👏 In the mean time I've analyzed and tested the track completely no unknown fields anymore 🥳 Going to analyze the pattern footer now.

sU8 __unknown_027C[9]; part is:

[src/rytm.rs:156] unknown_bytes = [
    100, // Trig probability 0..=100
    0, // Euclidean mode on = 128 off = 0
    0, // Euclidean PL1 0..=64 (max depends on length, total max is 64 because total length max is 64)
    0, // Euclidean PL2 0..=64 (max depends on length, total max is 64 because total length max is 64)
    63, // RO1  0..=126 (middle point 63)
    63, // RO2  0..=126 (middle point 63)
    63, // TRO 0..=126 (middle point 63)
    255, // PAD SCALE 255 chromatic 0..=34 All modes
    96, // Root Note 96..=107 From C to B
]

The modes in order for PAD_SCALE

# • IONIAN (MAJOR)
# • DORIAN
# • PHRYGIAN
# • LYDIAN
# • MIXOLYDIAN
# • AEOLIAN (MINOR)
# • LOCRIAN
# • PENTATONIC MINOR
# • PENTATONIC MAJOR
# • MELODIC MINOR
# • HARMONIC MINOR
# • WHOLE TONE
# • BLUES
# • COMBO MINOR
# • PERSIAN
# • IWATO
# • IN-SEN
# • HIRAJOSHI
# • PELOG
# • PHRYGIAN DOMINANT
# • WHOLE-HALF DIMINISHED
# • HALF-WHOLE DIMINISHED
# • SPANISH
# • MAJOR LOCRIAN
# • SUPER LOCRIAN
# • DORIAN b2
# • LYDIAN AUGMENTED
# • LYDIAN DOMINANT
# • DOUBLE HARMONIC MAJOR
# • LYDIAN #2 #6
# • ULTRAPHRYGIAN
# • HUNGARIAN MINOR
# • ORIENTAL
# • IONIAN #2 #5
# • LOCRIAN bb3 bb7

Sorry for posting dirty analysis data but I think it conveys the info 😉

alisomay commented 9 months ago

I'll check the ?@ marks for the ranges now.

bsp2 commented 9 months ago

very nice, thanks! I've just pushed another update that merges your findings.

alisomay commented 9 months ago

Looks great! By the way it is an in progress update but I'm analyzing the pattern footer now.

__unknown3327 = 0 // Swing amount, 0..=30, 0 is 50%, 30 is 80%

and let's see what will we find in the old swing amount's place.. 😆

bsp2 commented 9 months ago

hmm, but the swing amount is @3326 according to my tests (?!)

just tested it again, it's 0x3326

alisomay commented 9 months ago

Thanks for letting me know, I might be missing allignment in the footer by one byte I'm double checking now.

bsp2 commented 9 months ago

that's why I always write these "@0x1234" offsets in the comments, so easy to misalign things.

0x3327 (still) is

sU8                time_mode;        /* @0x3327            <void> timeMode (0=normal, 1=advanced) */
alisomay commented 9 months ago

Now I double checked carefully, I confirm that I misalligned by a byte in the pattern footer. Phew.. Glad that you pointed out early.

bsp2 commented 9 months ago

btw: I really do wonder what made them change the trigs array from u16 to u14. It makes the sysex dump a whopping 1.6% (!) smaller and renders 3rd party SW incompatible (IIRC someone built an iPad app that's based on this library, and, most importantly, the SyxEx format). Seems unnecessary to me.

alisomay commented 9 months ago

I totally agree, I don't see a reason for that. Normally in companies people don't try to optimize working stuff when they don't have a very valid reason. It is a mystery.

alisomay commented 9 months ago

By the way the first two values of sU8 __unknown332A[4]; is still the bpm msb and lsb.

((footer[8] as u16) << 8 | (footer[9] as u16) << 0) / 120 would give you the bpm value like the old format. (Rust snippet)

I just tested it.

bsp2 commented 9 months ago

I've commented the bpm fields earlier today b/c I thought that the BPM is not stored per pattern anymore. So there still is an option for this ? (I've forgotten a few things over the years, it is quite a complex piece of machinery).

alisomay commented 9 months ago

Yep in the bpm menu you can choose PTN or PROJ. It looks like that you can still store the bpm for a pattern.

Only

I got hooked :)

bsp2 commented 9 months ago

good to know, will revert the change in that case (just skimmed the manual and did not find it by searching for "bpm")

bsp2 commented 9 months ago

final pattern.h update for today (also fixes a 1-byte shift after the plock_seq array).

alisomay commented 9 months ago

It was fun and productive ✨ It is much easier to do these things together thanks for the company. I might still post some stuff to this thread but it does not mean I'd expect an update or an answer immediately.

I found out that __unknown3325 is the kit number 0..=127 btw.

Have a good rest and a week!

bsp2 commented 9 months ago

..and quickly checked the p-locks: they are indeed at 0x2091 and the format appears to remain the same. The AR_PLOCK_TYPE_* enum also seems to still be valid (tested machine params 0+1 and LFO phase (0x26)).

bsp2 commented 9 months ago

yeah you too have a good one. gotta be back at actual work in a few hours :D

alisomay commented 9 months ago

Further findings.

For __unknown3321 and __unknown332C hm.. They are always 0 and 1. I have traversed the manual and played with the machine for an hour but couldn't find a way to effect them :)

Maybe it is fine if we leave them unknown for now. We can come back to them if we find something we have not covered in the pattern scope.

So suggested update: (something like this)

typedef struct { /* 0x332D(13101) bytes (v5/FW1.70), 0x3395 bytes (v4 / FW1.50..1.61b), 0x3386 bytes (v1) */
   sU8                magic_header[4];  /* ??? a version number ???
                                              reads '00 00 00 01' (v1) 
                                                and '00 00 00 03' (v4 / FW1.50)
                                                and '00 00 00 05' (v5 / FW1.70)
                                         */
   ar_pattern_track_t tracks[13];       /* @0x0004..0x2090? (0x281(641) bytes per track in v5/FW1.70, 0x289 bytes per track in v4/FW1.50/1.61b, 0x288 in v1) */
   ar_plock_seq_t     plock_seqs[72];   /* ?@0x2091..0x3320  16#42*72=4752 bytes                        */
   sU8                __unknown3321;    /* @0x3321                                                      */
   sU8                pattern_len;      /* @0x3322            Master length (in adv mode). 1=inf        */
   sU8                master_chg_msb;   /* @0x3323            masterChange MSB                          */
   sU8                master_chg_lsb;   /* @0x3324            masterChange LSB (1=OFF, 2=2, 3=3, ..,  ) */
   sU8                kit_number;       /* @0x3325            <void, ali> Reads 0xFF when kit is not saved. 0..127 otherwise. */
   sU8                swing_amount;     /* @0x3226            swingAmount (0..30 => 50%..80%)           */
   sU8                time_mode;        /* @0x3327            <void> timeMode (0=normal, 1=advanced)    */
   sU8                pattern_speed;    /* @0x3228            See AR_SPD_xxx.                           */
   sU8                global_quantize;  /* @0x3229                                                      */
   sU8                bpm_msb;          /* @0x332A multiplied by 120. (used when BPM mode=PTN)          */
   sU8                bpm_lsb;          /* @0x332B                                                      */
   sU8                __unknown332C; /* @0x332C */
} ar_pattern_t;

Good night :sparkle:

bsp2 commented 9 months ago

tonight I have..

p.s.: the byte at 0x3321 is probably just the MSB of the master_length field. A lot of the data in the sysex dump is just zeroes because bytes are often stored as 16 bit shorts (just take a look at sound.h and kit.h. At least a few of these bytes have purpose now (since FW1.70). "Yay" for 16bit STA / END :-)

alisomay commented 9 months ago

Great work! 👏 Yesterday night I was on the Rust wrapper, making the necessary convenience side to work with the packed structs which comes from the libanalogrytm bindings, wrapper types and test the response of pattern generation. It is a dumb randomization for testing nothing musical but it works perfectly 🥳 And the pattern read and writes are pretty responsive. (I'm trying writes for the first time.)

https://github.com/bsp2/libanalogrytm/assets/16831945/d428783f-ac06-4891-85ed-bdd52a822815

It is great that you've written helpers for the C developers! There is a nice crate in Rust already https://docs.rs/bitstream-io/latest/bitstream_io/ which I use for 14 bit reading and writing which simplifies things.

Excerpts.

Read:

        let trig_cursor = Cursor::new(raw_track.trig_bits);
        let mut bit_reader = BitReader::endian(trig_cursor, BigEndian);

        // TODO: Handle Errors
        for trig in &mut trigs {
            *trig = Trig::from(bit_reader.read::<u16>(14).unwrap());
        }

Write:

        let mut bit_writer = BitWriter::endian(Cursor::new(trig_bits.as_mut_slice()), BigEndian);
        for trig_flag in trig_flags {
            bit_writer.write::<u16>(14, trig_flag).unwrap();
        }

I'll give the master_length idea a try tonight to double test it and play with kit and sound requests ✨

bsp2 commented 9 months ago

why use another library when the lib already contains bitstream reader / writer functions ?

but well, your choice. just pushed the final update for now (mainly clean up).

unless something is terribly wrong this will be it for a while (just made a small demo-track using my kit editor + pattern importer and it appears to work fine).

alisomay commented 9 months ago

That was before you have added the helper functions actually.

On the other hand when writing Rust it is generally a better idea to interface with the C world for only the essentials to avoid the necessity of using raw pointers and C like types as much as possible and in our case working with packed structs.

My general strategy is to use libanalogrytm for the essential encoding/decoding and raw type information then use my own structs (rust structs which have a different alignment than C structs) so I can comfortably work with them in Rust with no restrictions.

When the encoding time comes, I convert them to the packed struct types you provide and serialize them to a buffer to send back to elektron.

I could have chosen to translate libanalogrytm to pure Rust also instead of generating the bindings but I think that would be a bad choice. Because I think having one central library to define essential types, encoding/decoding methods is one thing to maintain and one source of information which is more convenient. Then any language which can interface with C would use it.

Now I'll also spend some tonight to go over the master_length thing and check out kits and sounds. My implementations also work like a clock, I'm quite happy with the current state.

bsp2 commented 9 months ago

in my SW there are three layers: 1) libanalogrytm (structs and enums, SysEx encoding / decoding, 14bit trig array access) 2) C++ wrapper around libanalogrytm, adds some utility functions (accessible from scripts) 3) Script classes (Pattern / Kit / Sound / ..) (utility + debug methods, stream i/o, UI helpers, ..)

Additional functionality is also implemented as scripts (you can, for example, map trig button combos and pads to actions like sending patterns / kits / sounds, recall or save performance controller states and transitions, mute states, emit MIDI events, and so on)

anyways, that's a whole other topic. Glad to hear you also haven't found any issues with the current state of the SysEx format docs / structs.

alisomay commented 9 months ago

Yes! I also just tested the master_length msb idea and that is indeed the case I can confirm.

 println!(
   "Maybe master len: {}",
   ((rytm.patterns[0]._unknown_3321 as u16) << 8) | rytm.patterns[0].master_len as u16
 );

This exactly matches when I update the master_length on the device.

Now trying to come up with the idea for the last piece of the puzzle _unknown_332c