Next set of split_common changes #4974

pelrun · 2019-01-28T05:12:03Z

Description

Right now this PR does the following things with the split_common code:

Eliminate the BACKLIT_DIRTY/RGB_DIRTY flags in favour of detecting value changes implicitly.
Port the i2c transport over to the standard i2c_master.c/i2c_slave.c
Eliminate the MATRIX_COLS<=8 restriction for i2c splits

Types of changes

Checklist:

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document. (https://docs.qmk.fm/#/contributing)
I have added tests to cover my changes.
I have tested the changes and verified that they work and don't break anything (as well as I can manage).

drashna · 2019-01-28T05:28:48Z

3. Eliminate the MATRIX_COLS<=8 restriction for i2c splits

Ooooh, wait, this means that boards with 9+ columns can be posted over?
Eg, I can finally port the orthodox over, and the BFO9000?

2. Port the i2c transport over to the standard i2c_master.c/i2c_slave.c

Woooooo

pelrun · 2019-01-28T05:33:19Z

The only reason it was restricted was because it sent 1 byte per row. I just made it pay attention to sizeof(matrix_row_t).

Of course, as I don't actually have an AVR split kb, all these changes are done essentially blind. Testing needed!

drashna · 2019-01-28T06:03:18Z

Well, I can confirm that it works over serial on the Orthodox (9 columns).

pelrun · 2019-01-28T06:19:48Z

Ha! Serial always worked.

drashna · 2019-01-28T07:04:39Z

Not with more than 8 columns. :)

But I'd like to see somebody with i2c test this out, before merging it.

@nooges ?

pelrun · 2019-01-28T07:42:42Z

Well, it had absolutely nothing to do with my changes (I've not changed serial at all), it must have been fixed with #4669.

drashna · 2019-01-28T07:59:21Z

Ah, it was a couple of changes, actually. But either way... it should work on both i2c and serial now.

quantum/split_common/transport.c

drashna · 2019-01-28T23:14:57Z

@mtei With #4984 merged, any other feedback on this?

drashna · 2019-01-30T01:19:53Z

@nooges Would you mind testing this?

It affects i2c, and I have no i2c boards to test and verify.

Lenbok · 2019-01-30T07:57:34Z

I just tested this on my i2c redox split and it doesn't work for me. I'm using EE_HANDS and only the side that the USB is plugged into works.

As a side note, I flashed back to master (0.6.263) and both sides are working, however I notice that compared to my previous working firmware (based on 0.6.233) RGB underglow animations are now playing on both sides -- when did that happen? It's a bit buggy in terms of the sides getting out of sync and animations cause dropped keypresses. Is there an issue / PR tracking progress on i2c split animations?

pelrun · 2019-01-30T08:07:52Z

Okay. I'll bite the bullet and set up some hardware to debug this properly.

drashna · 2019-01-30T08:26:54Z

To make sure, you flashed both sides when testing?

if not, you may need to.

Lenbok · 2019-01-30T08:31:38Z

@drashna yep, I flashed both.

pelrun · 2019-02-23T05:00:09Z

Hmm. I finally found time to set up a couple of pro-micros on breadboard and flashed them with redox:default, and the slave is working fine.

Lenbok · 2019-02-23T07:03:00Z

@pelrun redox:default uses serial, did you change it to i2c? (Mine is wired/configured for i2c.)

pelrun · 2019-02-23T07:19:30Z

Whoops, been away from this a bit too long, I think. Adding USE_I2C breaks it, at least now I can debug.

Lenbok · 2019-02-23T07:24:16Z

quantum/split_common/transport.c

+
+#  ifdef RGBLIGHT_ENABLE
+  static uint32_t prev_rgb = ~0;
+  uint32_t        rgb      = eeconfig_read_rgblight();


Does this need to be read out of eeprom rather than directly using the rgblight_config struct that the master is currently running?

I've recently implemented an inactivity timeout for rgb underglow like this:
https://gist.github.com/Lenbok/603c66903c048f6879ee0860b37f8a7b
Ideally I'd like to use rgblight_toggle_noeeprom() when the idle timer is responsible for toggling things (both to avoid eeprom writes and also as the inactivity based status probably shouldn't be written to eeprom anyway), but found that I could only get the toggle to get sent to the slave side by using the regular/eeprom version of rgblight_toggle().

There's nothing requiring eeprom access, in fact I'd rather there be none. If there's a function to get the config directly (I don't want to be reaching into variables directly) then that can go in without issue... it's just not something I'm concerned with right now.

pelrun · 2019-03-08T22:55:32Z

11 columns on the right side, and 7 on the left

Yikes. The code expects the matrix to be identically sized on both sides. You'll have to post your configuration.

Gimly · 2019-03-09T09:40:19Z

@pelrun I made it identically sized by adding 4 fake columns on the left, which I added to the UNUSED_PINS array.

I have to compile two different versions because the left and right side don't use the same pins on the microcontroller.

Also because I've used Teensy 2 and the master / slave detection doesn't work for teensy.

You can check my code there https://github.com/Gimly/qmk_firmware/tree/gimly-split/keyboards/gimly_split

I've rebased my branch upon the one used for this pr.

nooges · 2019-03-09T15:07:46Z

@Gimly You can use MATRIX_ROW_PINS_RIGHT/MATRIX_COL_PINS_RIGHTto define different pinouts for each half as seen here: https://github.com/qmk/qmk_firmware/blob/master/keyboards/keebio/quefrency/rev1/config.h#L37-L40

Lenbok · 2019-03-09T21:42:13Z

Since the Teensy master/slave stuff seems out of the scope of this PR and there similarly wasn't any expectation you could have different MATRIX_COLS size on each side, it sounds like this PR is actually fine with regard to the issues @Gimly had. Perhaps once @nooges has a chance to test and sign off we're good to merge?

nooges · 2019-03-10T05:30:58Z

@drashna @pelrun Okay, finally got time to test things out with serial and I2C, and verified that LED and RGB changes work properly over each half. Guess I can dust off my work on splitting RGB LEDs between the halves for animations now.

nooges · 2019-03-10T06:48:36Z

Whoops, one thing I did notice is that the RGB mode does not sync between halves in serial mode. Can't remember if that's a regression or if it hadn't been done before, I'm guessing probably the latter.

mtei · 2019-03-10T08:30:39Z

From July 2018 until now, I think that implementation has not been completed for rgblight.

a012113#diff-2615833595f67363eda3c008b8f3f690

drashna · 2019-03-11T02:46:19Z

@mtei You're right. IIRC, we've talked about it elsewhere about it not being implemented fully, and moving to implement it.

@nooges Awesome. If serial isn't working, it wasn't working before, and I don't think that this PR changes that behavior.

Unless there is any objections, I'd like to merge this PR "as is", and the next set of changes can be worked on, including getting RGB Light working on serial properly, rgb animations over i2c/serial, and ARM support. As well as whatever else.

And again, thanks all for all of the work! You all are awesome!

nooges · 2019-03-11T18:41:38Z

I'm good with that.

* 'master' of https://github.com/qmk/qmk_firmware: (48 commits) [Keymap] Added Boy_314's layout for half n half keyboard (qmk#5373) Align use of atmega32a program script (qmk#5259) [Keyboard] new keyboard lovelive9 (qmk#5266) [Keyboard] Inital port of xd96 (qmk#5401) Fix ascii art (qmk#5407) [Keyboard] Georgi Support (qmk#5384) fresh commit for a new fork for PR to upstream/master (qmk#5406) Added info.json for mt980 keyboard and fixes to walker keymap (qmk#5391) Add support for THE60 (qmk#5385) Added 1up60rgb keymap: mdyevimnav (qmk#5386) Fix i2c calls for HotDox keyboard (qmk#5387) Sleep until USB port becomes writable before running avrdude (qmk#5393) [Keymap] Some more improvements to keymap, currency symbols.. (qmk#5395) [Keymap] Add atreus, ergotravel and org60 keymaps (qmk#5381) archetype keymap for jj50 (qmk#5397) Wheat Field Peripherals mt980 (FC980M Layout) PCB Support (qmk#5374) Minor readme fix (qmk#5389) Add new keyboard Plaid and ATMEGA328p support (qmk#5379) Next set of split_common changes (qmk#4974) [Keyboard] Lily58 Add info.json file (qmk#5354) ...

* Update split_common to use standard i2c drivers * Eliminate RGB_DIRTY/BACKLIT_DIRTY * Fix avr i2c_master error handling * Fix i2c_slave addressing * Remove unneeded timeout on i2c_stop() * Fix RGB I2C transfers * Remove incorrect comment

drashna added enhancement optimization core labels Jan 28, 2019

drashna mentioned this pull request Jan 28, 2019

Convert Orthodox to Split Common code #4975

Merged

13 tasks

mtei reviewed Jan 28, 2019

View reviewed changes

quantum/split_common/transport.c Show resolved Hide resolved

This was referenced Jan 31, 2019

Add function to support split-keyboard in rgblight.[ch]. #5020

Merged

Split i2c animations cause random key losses and insertions #5037

Closed

drashna added in progress on hold labels Feb 15, 2019

pelrun added 2 commits February 23, 2019 14:52

Eliminate RGB_DIRTY/BACKLIT_DIRTY

3ff6bf3

Update split_common to use standard i2c drivers

8e9dcc0

Lenbok reviewed Feb 23, 2019

View reviewed changes

pelrun added 2 commits February 23, 2019 23:16

Fix avr i2c_master error handling

0925c56

Fix i2c_slave addressing

dda1e4f

drashna merged commit 37932c2 into qmk:master Mar 12, 2019

mtei mentioned this pull request Mar 13, 2019

Fix build break make hotdox:default and make hotdox:bocaj #5394

Closed

13 tasks

pelrun deleted the split_enhance branch March 13, 2019 11:29

zvecr mentioned this pull request Mar 15, 2019

XD84 no longer detecting key presses / ARM I2C behaviour recently changed #5410

Merged

13 tasks

nooges mentioned this pull request Jun 19, 2019

Bad Lag sometimes happens with I2C on split boards #5569

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Next set of split_common changes #4974

Next set of split_common changes #4974

pelrun commented Jan 28, 2019 •

edited

Loading

drashna commented Jan 28, 2019

pelrun commented Jan 28, 2019

drashna commented Jan 28, 2019

pelrun commented Jan 28, 2019

drashna commented Jan 28, 2019

pelrun commented Jan 28, 2019 •

edited

Loading

drashna commented Jan 28, 2019

drashna commented Jan 28, 2019

drashna commented Jan 30, 2019

Lenbok commented Jan 30, 2019

pelrun commented Jan 30, 2019

drashna commented Jan 30, 2019

Lenbok commented Jan 30, 2019

pelrun commented Feb 23, 2019

Lenbok commented Feb 23, 2019

pelrun commented Feb 23, 2019

Lenbok Feb 23, 2019

pelrun Feb 23, 2019

pelrun commented Mar 8, 2019

Gimly commented Mar 9, 2019

nooges commented Mar 9, 2019

Lenbok commented Mar 9, 2019

nooges commented Mar 10, 2019

nooges commented Mar 10, 2019 •

edited

Loading

mtei commented Mar 10, 2019

drashna commented Mar 11, 2019

nooges commented Mar 11, 2019

Next set of split_common changes #4974

Next set of split_common changes #4974

Conversation

pelrun commented Jan 28, 2019 • edited Loading

Description

Types of changes

Checklist:

drashna commented Jan 28, 2019

pelrun commented Jan 28, 2019

drashna commented Jan 28, 2019

pelrun commented Jan 28, 2019

drashna commented Jan 28, 2019

pelrun commented Jan 28, 2019 • edited Loading

drashna commented Jan 28, 2019

drashna commented Jan 28, 2019

drashna commented Jan 30, 2019

Lenbok commented Jan 30, 2019

pelrun commented Jan 30, 2019

drashna commented Jan 30, 2019

Lenbok commented Jan 30, 2019

pelrun commented Feb 23, 2019

Lenbok commented Feb 23, 2019

pelrun commented Feb 23, 2019

Lenbok Feb 23, 2019

Choose a reason for hiding this comment

pelrun Feb 23, 2019

Choose a reason for hiding this comment

pelrun commented Mar 8, 2019

Gimly commented Mar 9, 2019

nooges commented Mar 9, 2019

Lenbok commented Mar 9, 2019

nooges commented Mar 10, 2019

nooges commented Mar 10, 2019 • edited Loading

mtei commented Mar 10, 2019

drashna commented Mar 11, 2019

nooges commented Mar 11, 2019

pelrun commented Jan 28, 2019 •

edited

Loading

pelrun commented Jan 28, 2019 •

edited

Loading

nooges commented Mar 10, 2019 •

edited

Loading