New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
emallson opened this Issue Jan 13, 2016 · 8 comments

Comments

Projects
None yet
4 participants
@emallson

emallson commented Jan 13, 2016

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

This comment has been minimized.

Show comment
Hide comment
@emallson

emallson Jan 13, 2016

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.

emallson commented Jan 13, 2016

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

This comment has been minimized.

Show comment
Hide comment
@haata

haata Jan 13, 2016

Member

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
#83 (comment).

Member

haata commented Jan 13, 2016

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
#83 (comment).

emallson added a commit to emallson/controller that referenced this issue Jan 13, 2016

Added (not 100% working) LCDStatus capability.
This commit has the problem in issue #83 on the main repository.

emallson added a commit to emallson/controller that referenced this issue Jan 13, 2016

@emallson

This comment has been minimized.

Show comment
Hide comment
@emallson

emallson Jan 13, 2016

@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.

emallson commented Jan 13, 2016

@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

This comment has been minimized.

Show comment
Hide comment
@alienth

alienth Jan 26, 2016

@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.

alienth commented Jan 26, 2016

@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

This comment has been minimized.

Show comment
Hide comment
@emallson

emallson Jan 27, 2016

@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)

emallson commented Jan 27, 2016

@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

This comment has been minimized.

Show comment
Hide comment
@alienth

alienth Jan 27, 2016

@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.

alienth commented Jan 27, 2016

@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

This comment has been minimized.

Show comment
Hide comment
@emallson

emallson Jan 27, 2016

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

emallson commented Jan 27, 2016

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

@eblanton

This comment has been minimized.

Show comment
Hide comment
@eblanton

eblanton May 7, 2016

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.

eblanton commented May 7, 2016

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment