SDDrive seems broken?

[jmamma]

@yatli

Snapshot restore is broken.

[jmamma]

Tried dropping midi speed to 1x.

Get SYSEX error code 1. Globals are all messed up.

[jmamma]

Some patterns completely erased ?

[jmamma]

Just tested a complete rec/dump using C6 and the TM-1. No problems. So problem is not on the MD side.

[jmamma]

MD.setSysexRecPos()

implementation doesnt look correct.

edit: looks okay.

[jmamma]

Just checked 2.70 . No problems with X.03

commit b923b202d187df5bd879bd0a49a38c268e9f412b
Author: Yatao Li <yatli@microsoft.com>
Date:   Wed Sep 2 20:43:14 2020 +0800

    super-charged refactor: unify ElektronDevice implementations

I'm guessing this broke sysex transmission ^ @yatli

[jmamma]

Won't be able to release 3.0 until this one is sorted.

https://github.com/jmamma/MIDICtrl20_MegaCommand/commit/b923b202d187df5bd879bd0a49a38c268e9f412b

[jmamma]

Kit and Global transmission is okay. It's the pattern transmission that is broken.

[jmamma]

MD SYSEX verify is not showing any transmission errors. so length + checksum calc must be okay, but payload is bad.

[jmamma]

 91   MDPattern() : ElektronPattern() {
 92     maxSteps = 64;
 93     maxParams = 24;
 94     maxTracks = 16;
 95     maxLocks = 64;
 96 
 97     isExtraPattern = false;
 98     init();
 99   }
100 
101   /* XXX TODO extra pattern 64 */
102 

^^ @yatli TODO ??

[jmamma]

  uint8_t i = 0;
  MD.getBlockingPattern(i);

  MD.setSysexRecPos(8, i + 1); 
  delay(20);
  MD.pattern.toSysex();

This is test routine is working ^ so encode /decode must be okay.

Problem must be SD storage? templates perhaps?

[jmamma]

 21   template <class T> bool read_data_v(T *data, FatFile *filep) {
 22     auto ret = read_data(data, sizeof(T), filep);
 25     ::new(data)T;
 26     return ret;
 27   }

::new calls the constructor.

i.e isExtraPattern = false; 

[jmamma]

Called by the constructor!

43     /** Clear the pattern. */
 44     void init() {
 45         clearPattern();
 46     }   

[yatli]

What happened today 0.0

[jmamma]

more 🐛 🐞

[yatli]

dev branch? I'll take a look

[jmamma]

no, master.

[jmamma]

the problem is the constructor call.. in

21 template bool read_data_v(T data, FatFile filep) { 22 auto ret = read_data(data, sizeof(T), filep); 25 ::new(data)T; 26 return ret; 27 }

it indirectly calls clearPattern();

[yatli]

Only happens for MDPattern?

[jmamma]

yes

[jmamma]

Was there a vtable in previous versions of MDPattern.. if not, the schema has changed and old .snp's won't be compatible

[yatli]

clearPattern is called even twice 0.0

[yatli]

I just checked 2.70 -- the inheritance tree is intact -- MDPattern -> ElektronPattern And the same init sequence: MDPattern ctor calls ElektronPattern ctor calls clearPattern, then MDPattern ctor calls init()

The twist is... in 2.70, MDPattern::clearPattern is not explicitly called, and MDPattern::clearPattern is not declared as virtual, and thus base instead calls the base implementation clearPattern() {}, so the pattern isn't cleared at all 😱

TL;DR

I fixed a bug where the derived class doesn't properly declare virtual methods, and then it backfired (

[yatli]

@jmamma dinner time. Will fix this 0.5~1.0 hrs later

[yatli]

https://github.com/jmamma/MIDICtrl20_MegaCommand/pull/150