kiibohd / controller

Kiibohd Controller
GNU General Public License v3.0
807 stars 270 forks source link

"Too much data to send on UART0" from Left -> Right but not Right -> Left #83

Open emallson opened 8 years ago

emallson commented 8 years ago

I have implemented a custom capability (to turn off the LCD backlight; code below) on my Infinity ErgoDox. When I have the right hand as master, it works fine: both halves turn off the LCD backlight. When I use the left hand as master, the keyboard locks up and the message

WARNING - Too much data to send on UART0, waiting...

appears over and over in the log (read from serial port with screen). I have this capability bound to Fun3 Delete, both of which are on the LHS. I'm running the latest commit of this repository and flashed both halves of the board after rebuilding just to make sure there wasn't a version mismatch.

This is the capability (added to lcd_scan.c):

uint8_t LCD_status = 1; // start in the ON state
void LCD_status_capability( uint8_t state, uint8_t stateType, uint8_t *args ) {
  uint8_t status = *args;
  if(state == 0xFF && stateType == 0xFF) {
    print("LCD_status_capability(status)");
  }

  // Only deal with the interconnect if it has been compiled in
#if defined(ConnectEnabled_define)
    if ( Connect_master )
    {
        // generatedKeymap.h
        extern const Capability CapabilitiesList[];

        // Broadcast layerStackExact remote capability (0xFF is the broadcast id)
        Connect_send_RemoteCapability(
            0xFF,
            LCD_status_capability_index,
            state,
            stateType,
            CapabilitiesList[ LCD_status_capability_index ].argCount,
            &status);
    }
#endif

  // stateType 0: normal
  // state 1: press (e.g. onKeyDown)
  if ( status == 0 ) {
    FTM0_C0V = 0;
    FTM0_C1V = 0;
    FTM0_C2V = 0;
    LCD_status = status;
  } else if ( status == 1) {
    LCD_layerStackExact_args stack_args;
    stack_args.numArgs = LCD_layerStackExact_size;
    memcpy(stack_args.layers, LCD_layerStackExact, sizeof(LCD_layerStackExact));
    LCD_layerStackExact_capability( state, stateType, (uint8_t*)&stack_args );
    LCD_status = status;
  } else if ( status == 2 && stateType == 0x00 && state == 0x03 ) {
    status = !LCD_status;
    LCD_status_capability( state, stateType, &status );
  }
}

I defined the capability in STLcd/capabilities.kll as:

# set LCD status. 1 = On, 0 = Off, 2 = Toggle
LCDStatus            => LCD_status_capability( status : 1 );

And then I call it as:

U"Delete": LCDStatus( 2 );

in a map appearing in PartialMaps[3].

I am unsure why this could be occurring. I remember it working last night right after I finished it up and tested it, but I do not know if it was plugged in as RHS master or LHS master.

emallson commented 8 years ago

I changed the code to only Connect_send_RemoteCapability in the non-recursive case (status == 0 or 1) and the problem went away.

Still not sure why it was manifesting in this way. The solution was non-obvious and I found it by trial and error.

haata commented 8 years ago

Interesting, do you have a fork that I can take a look at?

Also, something to keep in mind, you need to flash both sides when messing around with interconnect related code (some people forget this). Just before the first units were shipped I made a change to use DMA UART buffers so there shouldn't be any speed issues unless you are really pushing a lot of traffic at one time. If this really is the case, we can increase the buffer depth.

On Wed, Jan 13, 2016 at 12:28 PM J David Smith notifications@github.com wrote:

I changed the code to only Connect_send_RemoteCapability in the non-recursive case (status == 0 or 1) and the problem went away.

Still not sure why it was manifesting in this way. The solution was non-obvious and I found it by trial and error.

— Reply to this email directly or view it on GitHub https://github.com/kiibohd/controller/issues/83#issuecomment-171423059.

emallson commented 8 years ago

@haata the version that produces that problem is here. The fix is the commit after.

I didn't include my layouts (remapped colemak + thumb clusters); don't think they are part of the issue but I haven't checked to see if the problem occurs with the normal layout.

EDIT: I did also flash both halves each time.

alienth commented 8 years ago

@emallson Found something very curious with regards to this. On my ergodox with your fork, the LCDStatus toggle only seems to work if the toggle key is on the non-master side. For example, I've defined keys on the left and the right for the toggle. If the left is master, the right-side key does disable the LCD, but the left-side key does not. Vice versa if the right is master. I've verified my KLLs are setup correctly. Not seeing any errors in debug or experiencing any lockups.

emallson commented 8 years ago

@alienth that's on 5ab2deae8b? That is really weird...that's the version I have flashed (git status shows only kll, the ICED-*, and the modifications to my ergodox.bash) and it works for me with the toggle on the master side (master is left, key is Mod3 + Del, Del and Latch3 are both on Left)

Are you using LCDStatus( 0 ) or LCDStatus( 2 )? (0 is turn off, 2 is toggle)

alienth commented 8 years ago

@emallson Yessir. 'git diff' shows me the same changes for me.

I'm using LCDStatus(2):

./ICED-R/MDErgo1-Default-2.kll:U"RGUI" : LCDStatus(2);
./ICED-R/MDErgo1-Default-2.kll:U"LGUI" : LCDStatus(2);
./ICED-L/MDErgo1-Default-2.kll:U"RGUI" : LCDStatus(2);
./ICED-L/MDErgo1-Default-2.kll:U"LGUI" : LCDStatus(2);

And my diff:

$ gd -a
diff --git a/Keyboards/ergodox.bash b/Keyboards/ergodox.bash
index 85f85a2..7fedbf4 100755
--- a/Keyboards/ergodox.bash
+++ b/Keyboards/ergodox.bash
@@ -23,7 +23,7 @@ BaseMap="defaultMap leftHand slave1 rightHand"
 # This is the default layer of the keyboard
 # NOTE: To combine kll files into a single layout, separate them by spaces
 # e.g.  DefaultMap="mylayout mylayoutmod"
-DefaultMap="mdergo1Overlay lcdFuncMap"
+DefaultMap="MDErgo1-Default-0 lcdFuncMap"

 # This is where you set the additional layers
 # NOTE: Indexing starts at 1
@@ -31,8 +31,9 @@ DefaultMap="mdergo1Overlay lcdFuncMap"
 # e.g.  PartialMaps[1]="layer1 layer1mod"
 #       PartialMaps[2]="layer2"
 #       PartialMaps[3]="layer3"
-PartialMaps[1]="iced_func"
-PartialMaps[2]="iced_numpad"
+PartialMaps[1]="MDErgo1-Default-1"
+PartialMaps[2]="MDErgo1-Default-2"
+PartialMaps[3]="MDErgo1-Default-3"

@emailson, if you believe this to be a separate issue, let me know and I'd be happy to create a different ticket.

emallson commented 8 years ago

@alienth no I think this is the same issue manifesting slightly differently. The problem was weird in the first place

eblanton commented 8 years ago

I think the changes in my branch full-uart-buffer-fix (on https://github.com/eblanton/controller) will fix this problem. I cannot test due to present lack of hardware, however.