More problems with DOUT_MATRIX with led_emu_id_offset=1

[Sauraen]

Hi TK,
 
I thought I'd make this a separate topic. This is for a different DOUT_MATRIX (though same synth) than the one that has swap_rows_columns=1 (it doesn't even have that parameter at all).
 
The first problem I noticed was that mirrored_row=1 didn't seem to do anything. That is, with writes to a bunch of LED ids, and then enabling the parameter, did not change which hardware LEDs lit up. But this is not a serious problem, it just means the assignment numbers will be different.
 
The more serious problem is that the second shift register's worth of LEDs seem not to work (it's 8x16):

# LED matrix
DOUT_MATRIX n=2   rows=8  inverted_sel=1 sr_dout_sel1=1 \
                          inverted_row=0 mirrored_row=0 sr_dout_r1=3 sr_dout_r2=4 \
                          led_emu_id_offset=1001

I made a spreadsheet of how the offset IDs were being assigned to the elements of the matrix:

	0	1	2	3	4	5	6	7	0	1	2	3	4	5	6	7
0	1120	1119	1118	1117	1116	1115	1114	1113	1112	1111	1110	1109	1108	1107	1106	1105
1	1104	1103	1102	1101	1100	1099	1098	1097 	1096	1095	1094	1093	1092	1091	1090	1089
2	1088	1087	1086	1085	1084	1083	1082	1081	1080	1079	1078	1077	1076	1075	1074	1073
3	1072	1071	1070	1069	1068	1067	1066	1065  	1064	1063	1062	1061	1060	1059	1058	1057
4	1056	1055	1054	1053	1052	1051	1050	1049 	1048	1047	1046	1045	1044	1043	1042	1041
5	1040	1039	1038	1037	1036	1035	1034	1033	1032	1031	1030	1029	1028	1027	1026	1025
6	1024	1023	1022	1021	1020	1019	1018	1017 	1016	1015	1014	1013	1012	1011	1010	1009
7	1008	1007	1006	1005	1004	1003	1002	1001	1000	999	998	997	996	995	994	993

 

However, only the LEDs on the left shift register (the first 0-7) would light; I extrapolated the numbers for the right side, but couldn't actually confirm them because a statement like EVENT_LED id=1112 [etc.] would do nothing. (I didn't actually try all 64 on the side that were working, just a few here and there to establish the pattern, and at the beginning and end.)

 

Swapping the shift registers in the DOUT_MATRIX definition as follows

# LED matrix
DOUT_MATRIX n=2   rows=8  inverted_sel=1 sr_dout_sel1=1 \
                          inverted_row=0 mirrored_row=0 sr_dout_r1=4 sr_dout_r2=3 \
                          led_emu_id_offset=1001

results in the mapping

	0	1	2	3	4	5	6	7	0	1	2	3	4	5	6	7
0	1136	1135	1134	1133	1132	1131	1130	1129	1128	1127	1126	1125	1124	1123	1122	1121
1	1120	1119	1118	1117	1116	1115	1114	1113	1112	1111	1110	1109	1108	1107	1106	1105 
2	1104	1103	1102	1101	1100	1099	1098	1097	1096	1095	1094	1093	1092	1091	1090	1089
3	1088	1087	1086	1085	1084	1083	1082	1081	1080	1079	1078	1077	1076	1075	1074	1073 
4	1072	1071	1070	1069	1068	1067	1066	1065	1064	1063	1062	1061	1060	1059	1058	1057
5	1056	1055	1054	1053	1052	1051	1050	1049	1048	1047	1046	1045	1044	1043	1042	1041
6	1040	1039	1038	1037	1036	1035	1034	1033	1032	1031	1030	1029	1028	1027	1026	1025
7	1024	1023	1022	1021	1020	1019	1018	1017	1016	1015	1014	1013	1012	1011	1010	1009

 

with now only the second shift register's worth of LEDs working and the first half not working. That is, the other 64 hardware LEDs now work but the first half don't (it's not a hardware problem).

 

So in both cases only the first-defined shift register will accept LED input, but the mapping is defined across both. In both cases the mapping is not correct either: in the first case it extends below 1001 into the non-working shift register, and in the second case, above 1128 into the non-working shift register.

 

If I can't find anything tonight, I'll try it with code from a new svn checkout just to make sure I didn't screw this up when I was fooling with mbng_matrix.c; but I'm pretty sure that all of the changes I made only executed additional code if swap_rows_columns=1 was active, and left the code untouched otherwise. Besides, I didn't change any code to do with the led_emu_offset_id mapping.

 

Thanks,
Sauraen

Edited January 13, 2014 by Sauraen

[TK.]

(I won't be able to support you on this topic the next days...)

 

Best Regards, Thorsten.

[Sauraen]

I can confirm that both of these problems with led_emu_id_offset--mirrored_row doing nothing and only the first-defined shift register's worth of LEDs working in an 8x16 matrix--exist in the unmodified midibox_ng_v1 (i.e. are actual bugs). I checked out a new copy and tried it again, with the same results. The mapping tables I wrote above also appear to be the same. I'm now going to switch back to the modified copy (so I can have my LED digits working) and see if I can actually fix the bug.

[Sauraen]

Bug squashed! Here's the changes:

 

In mbng_matrix.c, MBNG_MATRIX_DOUT_PinSet():

 

To make mirrored_row work right, right after int row, int column (your line 508), add:

//Feature added by Sauraen
if(m->flags.mirrored_row) column = (column&0xF8) | (7 - (column&7)); //Only swap lower 8 of column, as per spec

To fix the second-shift-register-not-working bug, right after the switch(color) statement (your original line 515), add:

//Bug fixed by Sauraen
column &= 7; //Above, we picked a different sr if column >= 8, but then we wrote to that sr with column still >= 8

 

One other bug I discovered while tracing the program flow is in mbng_dout.c, MBNG_DOUT_NotifyReceivedValue(), line 96, I'm pretty sure it should be

if( hw_id_ix < (m->led_emu_id_offset + (m->num_rows * row_size)) ) { //Bug? found by Sauraen

As it was, m->led_emu_id_offset was being multiplied by m->num_rows, and that looked like it was going to generate a huge number. This might never actually lead to a visible problem, since even though it would pass events with indices too high, at MBNG_MATRIX_DOUT_PinSet() it checks again (at original line 503) to make sure the given pin is in range. But it would have executed a couple hundred extra lines of code per LED event with large index.

[TK.]

Thank you very much, this was a bug^H^Hig help!  :thumbsup:

 

I integrated your changes and committed a new version to the SVN server.

 

Best Regards, Thorsten.

[Sauraen]

You're welcome! I hope this is only the first of the times my code ends up in the repository, heh heh!