Sauraen Posted January 3, 2014 Report Share Posted January 3, 2014 Hi TK, This is two independent issues I'm having, but there may be a single solution for the two of them. The first is that when I was designing the LED displays for my front panel, I misinterpreted the config file for the 7-segment displays example (code and diagram) and thought the "sel" DOUT shift register was connected to segments a-g, and the "r" (which evidently stands for "red", not "row") shift register to select which digit. So that's how I wired it, and obviously I'm getting all messed-up characters on the displays. It doesn't look like there would be an easy, existing software solution to this, but maybe there could be some way to swap the handling of rows and columns when writing to the LED matrix driving the digits. The second is that the way I have the buttons connected, I use "sel" wires as a lower-level than "r"/"din" wires--that is, for instance, softkeys 1-8 are all hooked to a single DIN pin but each to the eight different DOUT selection drivers. When I use button_id_emu_offset or led_id_emu_offset to convert them into individual button or LED handlers, those softkeys appear as buttons 1001, 1009, 1017, 1025, etc. instead of 1001, 1002, 1003, 1004, etc., because it's going through all the "r"/"din" wires for one "sel" wire first and then moving to the next "sel" wire (which makes sense on a hardware level). I know this is not a major deal and just means that my config files will not have all the buttons in a nice neat order, but again if there was some optional way to reverse the handling of rows and columns, it would make this easier. I'm thinking something like a "swap_rows_columns=<1|0>" parameter for DOUT_MATRIX and DIN_MATRIX definitions that would work like this. The hardware would still be configured as the sr_dout_sel* and sr_din* and sr_rgb statements in the definition suggested, this new parameter would not change that at all. However, if it was 1, any time any function wrote to (DOUT) or read from (DIN) an element in the matrix, the row and column indices given would be swapped before reading/writing. Might this be easy to add? If you want me to try adding it myself, I can try, I've already been fooling with adding a "type=MBFM MBFM=...", but other users might also be able to benefit from this ability to swap rows and columns. Thanks, Sauraen Quote Link to comment Share on other sites More sharing options...
TK. Posted January 3, 2014 Report Share Posted January 3, 2014 Argl! Who should ever document and understand all those options! ;-) The problem at my side: I can't test this, since my matrices are connected correctly. Therefore the implementation of a proper solution can take a lot of time (low prio for me) Best Regards, Thorsten. Quote Link to comment Share on other sites More sharing options...
Sauraen Posted January 4, 2014 Author Report Share Posted January 4, 2014 (edited) It shouldn't be hard to test, if you have a Button/LED Matrix that's actually used as a matrix and not individually labeled buttons. If you invert the button matrix, the LED matrix should react to button presses as if it was mirrored across the diagonal; and if you invert the LED matrix, it should actually look mirrored across the diagonal. I was intending this would be inverted at a software level: the same shift registers would still be sending and receiving the same data, with the same row select signals and all, just when it is all written to or read from, the row and column numbers would be switched. Also, all the parameters you have already are absolutely necessary and generally well-documented (absolutely better documented than much commercial stuff, heh heh!). MBNG is a behemoth (200 Kb in flash?!) that I have barely scratched the surface of, and it's kind of magical that it automatically detects, configures, and runs things that I spent hours writing custom code for ASIDITY's front panel to do. So thanks again for providing it! Edit: To be clear, I want to be able to swap all the rows with all the columns, i.e. mirror the matrix across the diagonal, not swap individual rows with other rows or something. Just a single binary option for the whole matrix. You know, like: void DOUT_Matrix_SetLEDState(u8 column, u8 row, u8 state){ u8 temp; //add this part if(swap_rows_columns){ temp = row; row = column; column = temp; } //sample of already-existing code if(state) matrix_contents[row] |= 1 << column; else matrix_contents[row] &= ~(1 << column); } Edited January 4, 2014 by Sauraen Quote Link to comment Share on other sites More sharing options...
ilmenator Posted January 4, 2014 Report Share Posted January 4, 2014 Well if you have a solution for your problem already then why don't you simply add that to your code and be done? It sounds like you know how to do it!? Quote Link to comment Share on other sites More sharing options...
Sauraen Posted January 4, 2014 Author Report Share Posted January 4, 2014 Well if you have a solution for your problem already then why don't you simply add that to your code and be done? It sounds like you know how to do it!? If TK doesn't have time to work on it I will certainly try. But the above (fake) code is just the actual swapping, I don't know where in the entire MIOS32 architecture that function would be anyway, and it would require modification of the matrix definitions and .ngc parser (to understand the new keyword). More importantly, I was hoping it could be a permanent part of MBNG for everyone to use; if I add it myself, I can't commit to svn, so I would be the only one to be able to use it. Quote Link to comment Share on other sites More sharing options...
ilmenator Posted January 4, 2014 Report Share Posted January 4, 2014 No offence meant here, but reading your first post again it seems that the problem you ran into here (and which requires the solution you pointed out) actually is a particular one, caused by misinterpretation of the available documentation. Maybe it is better to invest time and forces into the improvement of the documentation then, instead of blowing up the complexity of the system itself? Quote Link to comment Share on other sites More sharing options...
Sauraen Posted January 4, 2014 Author Report Share Posted January 4, 2014 (edited) ...Except that I now have a partially-completed synth that I would have to make substantial hardware changes to in order to get to work, if I can't make a relatively simple software solution. I also think that adding one more optional parameter to a config file format that already supports hundreds of them is hardly "blowing up the complexity". I understand that you may see this as "n00b makes mistake and asks dev to change entire software package to cater to his special needs", but since the parameters for inverting the rows and columns and mirroring the columns were deemed useful enough to the general public to include them, and this seems to be a pretty similar thing, I would think it could be similarly useful. If it is that much of a problem I'll just add it to my own, but since this is a community, I'd really rather help to develop code that everyone can use, and allow what was originally my mistake to turn into more general hardware support for others in the future. Isn't that the point of MBNG being community-developed? TK finding out from other people building hardware what features would help make their lives easier? Edited January 4, 2014 by Sauraen Quote Link to comment Share on other sites More sharing options...
ilmenator Posted January 4, 2014 Report Share Posted January 4, 2014 My point is that adding these features would make the rather complex configuration options even more difficult to grasp, leaving those willing to start in MIDIbox land even more confused than what they are already. Speaking of community, I guess part of the things to be considered here is how much the community would actually benefit from these additional parameters, if there is a single person with a proprietary hardware - and why shouldn't that use proprietary code to run? But, TK already answered this, so who am I to speak for him :smile: . It's just my personal take on this and I might be totally off... Quote Link to comment Share on other sites More sharing options...
TK. Posted January 4, 2014 Report Share Posted January 4, 2014 We should handle it like the swapped encoder pin issue - if more than 2..3 people encounter the problem, it makes sense to add this into the official firmware. Otherwise it's better to use a local workaround, and to wait what happens in the next months. Why? Because it could turn out, that the new parameter wasn't flexible enough, and that somebody else would need even more flexibility. Instead of adding "just another flag", it could be that a combined flag is the better choice, e.g. to enforce that conflicting configurations can't be selected. But if the "simple" flag was already published, it would be too late to change this, because it would affect the compatibility. Another reason could be, that I find a more clever idea for the implementation - again in this case it would be better to wait... In other words: I prefer to collect "use cases" before adding new features which I don't need myself to understand the potential issues which need to be solved, and to avoid unnecessary complexity. Back to your proposal: the code for LEDs is in MBNG_MATRIX_DOUT_PinSet and for patterns (ledrings, BPM digits, etc) in Hlp_DOUT_PatternTransfer You will notice that it's already quite complex. Even I would need several minutes to understand what I did several months ago, and where to change something to swap columns and rows. Especially since you will notice that I support MAX72xx as well. Which means, that I would have to ensure that the flag even works for this variant. And of course it would have to support all possible matrix variants, such as 4x8, 8x4, 8x8, 16x4, 16x8, 4x16, 8x16, etc... You see the problem at my side? Therefore please start with some experiments at your side. Alternatively, fix the PCB (cut tracks, solder wires) - could be faster ;) Best Regards, Thorsten. Quote Link to comment Share on other sites More sharing options...
Sauraen Posted January 4, 2014 Author Report Share Posted January 4, 2014 An hour of coding later: It wasn't the easiest, as you said I had to consider a number of cases. I had to add an additional bit flag in mbng_patch.h : mbng_patch_matrix_flags_t, copy-and-paste to make parameter handlers for DIN_MATRIX and DOUT_MATRIX in mbng_file_c.c, and then in mbng_matrix.c, add a couple lines to MBNG_MATRIX_DOUT_PinSet and MBNG_MATRIX_ButtonHandler to do the swapping, and finally a separate handling section to Hlp_DOUT_PatternTransfer. I noticed the MAX72xx stuff but had no idea what that was, so I left it alone (i.e. the swapping would not work with those chips). My own matrices are 8x16 and 8x8 so I had to make sure those worked, and I considered the 16x16 case, but I'm not sure if the cases with 4 rows or columns will work. (That's why I wanted you to do it! :P) I don't think my code is good enough to add to the repository at this point, but as you said, if a couple more people ask here about the same thing, I'll send you what I changed, you can fix it up a bit and include it in the release. Quote Link to comment Share on other sites More sharing options...
TK. Posted January 4, 2014 Report Share Posted January 4, 2014 Cool! Could you please document all changes in this thread, so that I don't have to start from zero if somebody else requests the same feature? Most simple way to display all changes: cd $MIOS32_PATH svn diff . Best Regards, Thorsten. Quote Link to comment Share on other sites More sharing options...
Sauraen Posted January 5, 2014 Author Report Share Posted January 5, 2014 (edited) Index: apps/controllers/midibox_ng_v1/src/mbng_matrix.c =================================================================== --- apps/controllers/midibox_ng_v1/src/mbng_matrix.c (revision 1912) +++ apps/controllers/midibox_ng_v1/src/mbng_matrix.c (working copy) @@ -505,6 +505,15 @@ int row = pin / row_size; int column = pin % row_size; + int temp; + + //Added by Sauraen + if(m->flags.swap_rows_columns){ + temp = row; + row = column; + column = temp; + //DEBUG_MSG("Swapped row (now %d) and column (now %d)", row, column); + } u8 sr; switch( color ) { @@ -573,6 +582,47 @@ return 0; } + u8 sr1, sr2; + switch( color ) { + case 1: sr1 = m->sr_dout_g1; sr2 = m->sr_dout_g2; break; + case 2: sr1 = m->sr_dout_b1; sr2 = m->sr_dout_b2; break; + default: sr1 = m->sr_dout_r1; sr2 = m->sr_dout_r2; break; + } + + //Added by Sauraen + if(m->flags.swap_rows_columns){ + u16 column = row; + u8 sr; + if(column && !sr1) return -2; + if(column >= 8 && !sr2) return -2; + + if(m->flags.mirrored_row){ + //Change column as if mirroring across 8 columns + column = (column & 0xFFF8) + (7 - (column & 8)); + } + + if(m->flags.inverted_row){ + matrix_pattern ^= 0xFFFF; + } + + if(column >= 8){ + sr = sr2; + column -= 8; + }else{ + sr = sr1; + } + sr -= 1; //Evidently, zero-indexed internally + + int i; + for(row=0; row<m->num_rows; row++){ + for(i=0; i<MIOS32_SRIO_NUM_DOUT_PAGES; i+=m->num_rows) { + MIOS32_DOUT_PagePinSet(row + i, 8*sr + column, matrix_pattern & 1); + } + matrix_pattern >>= 1; + } + //DEBUG_MSG("Just finished writing pattern at column %d sr %d out of %d rows", column, sr, m->num_rows); + return 0; + } + if( row >= m->num_rows ) return -2; // ignore @@ -581,19 +631,16 @@ matrix_pattern = ((u16)mios32_dout_reverse_tab[matrix_pattern >> 8] << 8) | (u16)mios32_dout_reverse_tab[matrix_pattern & 0xff]; } - u8 sr1, sr2; - switch( color ) { - case 1: sr1 = m->sr_dout_g1; sr2 = m->sr_dout_g2; break; - case 2: sr1 = m->sr_dout_b1; sr2 = m->sr_dout_b2; break; - default: sr1 = m->sr_dout_r1; sr2 = m->sr_dout_r2; break; - } + if( m->flags.inverted_row ) matrix_pattern ^= 0xffff; u8 sr1_value = matrix_pattern; u8 sr2_value = matrix_pattern >> 8; - + + + if( m->num_rows ) { if( sr1 ) { u8 sr_offset = sr1-1; @@ -791,8 +838,12 @@ int pin; u16 mask = (1 << 0); for(pin=0; pin<row_size; ++pin, mask <<= 1) - if( changed & mask ) - MBNG_MATRIX_NotifyToggle(matrix, (row * row_size) + pin, (button_row[matrix][row] & mask) ? 1 : 0); + //Modified by Sauraen + if( changed & mask ){ + MBNG_MATRIX_NotifyToggle(matrix, + (m->flags.swap_rows_columns) ? ((pin * m->num_rows) + row) : ((row * row_size) + pin), + (button_row[matrix][row] & mask) ? 1 : 0 ); + } } } Index: apps/controllers/midibox_ng_v1/src/mbng_file_c.c =================================================================== --- apps/controllers/midibox_ng_v1/src/mbng_file_c.c (revision 1912) +++ apps/controllers/midibox_ng_v1/src/mbng_file_c.c (working copy) @@ -1788,6 +1788,18 @@ flags.mirrored_row = value; + ////////////////////////////////////////////////////////////////////////////////////////////////Added by Sauraen + } else if( strcasecmp(parameter, "swap_rows_columns") == 0 ) { + int value; + if( (value=get_dec(value_str)) < 0 || value > 1 ) { +#if DEBUG_VERBOSE_LEVEL >= 1 + DEBUG_MSG("[MBNG_FILE_C:%d] ERROR invalid swap_rows_columns flag for %s n=%d ... %s=%s (only 0 or 1 allowed)\n", line, cmd, num, parameter, value_str); +#endif + return -1; // invalid parameter + } + + flags.swap_rows_columns = value; + //////////////////////////////////////////////////////////////////////////////////////////////// } else if( strcasecmp(parameter, "button_emu_id_offset") == 0 ) { if( (button_emu_id_offset=get_dec(value_str)) < 0 || button_emu_id_offset >= 4095 ) { @@ -1950,6 +1962,18 @@ flags.mirrored_row = value; //////////////////////////////////////////////////////////////////////////////////////////////// + } else if( strcasecmp(parameter, "swap_rows_columns") == 0 ) { + int value; + if( (value=get_dec(value_str)) < 0 || value > 1 ) { +#if DEBUG_VERBOSE_LEVEL >= 1 + DEBUG_MSG("[MBNG_FILE_C:%d] ERROR invalid swap_rows_columns flag for %s n=%d ... %s=%s (only 0 or 1 allowed)\n", line, cmd, num, parameter, value_str); +#endif + return -1; // invalid parameter + } + + flags.swap_rows_columns = value; + + //////////////////////////////////////////////////////////////////////////////////////////////// } else if( strcasecmp(parameter, "max72xx_enabled") == 0 ) { int value; if( (value=get_dec(value_str)) < 0 || value > 1 ) { Index: apps/controllers/midibox_ng_v1/src/mbng_patch.h =================================================================== --- apps/controllers/midibox_ng_v1/src/mbng_patch.h (revision 1912) +++ apps/controllers/midibox_ng_v1/src/mbng_patch.h (working copy) @@ -53,6 +53,7 @@ u8 inverted_row:1; u8 mirrored_row:1; u8 max72xx_enabled:1; + u8 swap_rows_columns:1; //Added by Sauraen }; } mbng_patch_matrix_flags_t; Just to be clear, this new swap_rows_columns does nothing to the MAX72xx driver, and I can't guarantee it supports all matrix sizes (though I see no reason it shouldn't, of course I could have missed something). Edited January 5, 2014 by Sauraen Quote Link to comment Share on other sites More sharing options...
Sauraen Posted January 13, 2014 Author Report Share Posted January 13, 2014 It seems that the mirrored_row=1 parameter in a DOUT_MATRIX doesn't do anything at all when the matrix is assigned to individual LEDs with led_emu_id_offset=1001. From looking at MBNG_MATRIX_DOUT_PinSet, it doesn't look like there's code to handle this case (except in the MAX72xx portion). It's possible I accidentally removed this when I was messing with mbng_matrix.c, but it doesn't appear to be the case from looking at the revision history above. I tried adding a line to reverse the columns if this flag was set, but then no LEDs lit at all. Can someone else with a working DOUT_MATRIX that uses led_emu_id_offset try adding the mirrored_row=1 parameter to their config file and see if the matrix gets mirrored? Quote Link to comment Share on other sites More sharing options...
TK. Posted January 13, 2014 Report Share Posted January 13, 2014 That's a mistake at my side, I've to consider this flag in MBNG_MATRIX_DOUT_PinSet as well. Best Regards, Thorsten. Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.