Skip to content
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

[Bug] Split with I2C protocol + Oled does not compile #21570

Closed
2 tasks
t0kies opened this issue Jul 21, 2023 · 6 comments
Closed
2 tasks

[Bug] Split with I2C protocol + Oled does not compile #21570

t0kies opened this issue Jul 21, 2023 · 6 comments

Comments

@t0kies
Copy link
Contributor

t0kies commented Jul 21, 2023

Describe the Bug

It seems like you can not compile a split keyboard using the I2C protocol when you add an Oled feature.

I was able to replicate this issue with multiple split keyboards that utilize the I2C by just enabling the Oled feature in the info.json or rules.mk.

Compiling: drivers/oled/oled_driver.c drivers/oled/oled_driver.c: In function 'oled_init': drivers/oled/oled_driver.c:292:10: error: implicit declaration of function 'is_keyboard_master' [-Werror=implicit-function-declaration] 292 | if (!is_keyboard_master()) { | ^~~~~~~~~~~~~~~~~~ cc1: all warnings being treated as errors [ERRORS]

Keyboard Used

No response

Link to product page (if applicable)

No response

Operating System

Fedora Linux 38

qmk doctor Output

Ψ QMK Doctor is checking your environment.
Ψ CLI version: 1.1.2
Ψ QMK home: /home/user/qmk_firmware
Ψ Detected Linux (Fedora Linux 38 (Workstation Edition)).
Ψ Git branch: HEAD
Ψ Repo version: 0.21.0
⚠ Git has unstashed/uncommitted changes.
Ψ - Latest HEAD: 2023-05-29 06:17:24 +1000 (5024370dd0) -- Merge branch 'develop'
Ψ - Latest upstream/master: None
Ψ - Latest upstream/develop: None
Ψ - Common ancestor with upstream/master: None
Ψ - Common ancestor with upstream/develop: None
Ψ All dependencies are installed.
Ψ Found arm-none-eabi-gcc version 12.2.0
Ψ Found avr-gcc version 12.2.0
⚠ We do not recommend avr-gcc newer than 8. Downgrading to 8.x is recommended.
Ψ Found avrdude version 6.4
Ψ Found dfu-programmer version 0.7.2
Ψ Found dfu-util version 0.11
Ψ Submodules are up to date.
Ψ Submodule status:
Ψ - lib/chibios: 2023-04-15 13:48:04 +0000 --  (11edb1610)
Ψ - lib/chibios-contrib: 2023-01-11 16:42:27 +0100 --  (a224be15)
Ψ - lib/googletest: 2021-06-11 06:37:43 -0700 --  (e2239ee6)
Ψ - lib/lufa: 2022-08-26 12:09:55 +1000 --  (549b97320)
Ψ - lib/vusb: 2022-06-13 09:18:17 +1000 --  (819dbc1)
Ψ - lib/printf: 2022-06-29 23:59:58 +0300 --  (c2e3b4e)
Ψ - lib/pico-sdk: 2023-02-12 20:19:37 +0100 --  (a3398d8)
Ψ - lib/lvgl: 2022-04-11 04:44:53 -0600 --  (e19410f8)
Ψ QMK is ready to go, but minor problems were found

Is AutoHotKey / Karabiner installed

  • AutoHotKey (Windows)
  • Karabiner (macOS)

Other keyboard-related software installed

none

Additional Context

This issue is specific to version 0.21 and above

@fauxpark
Copy link
Member

fauxpark commented Jul 21, 2023

This is expected behaviour. The slave MCU cannot simultaneously be slave to the master MCU, and master to its OLED. You can only use the serial transport if you want OLEDs.

@t0kies
Copy link
Contributor Author

t0kies commented Jul 21, 2023

@fauxpark

I was able to use oled before while using the i2c protocol with versions before .21.0

you can use an oled with a split that utlizes the i2c protocol however I don't think you can use 2 separate oleds.

your slave MCU can not control the oled but your master can for one of the halves (or both did not try that yet).

The issue was implicit declaration of function 'is_keyboard_master' inside the updated oled_driver.c

.drivers/oled/oled_driver.c
line 290
#if defined(USE_I2C) && defined(SPLIT_KEYBOARD) && defined(OLED_TRANSPORT_I2C)
Here it checks if i2c is used with a split and an oled with i2c as well

@fauxpark
Copy link
Member

I do see oled_driver.c may be missing #include "keyboard.h" for is_keyboard_master(). There've been some recent cleanups to reduce the usage of quantum.h (which basically includes everything) and keep includes to only what the files use.

@t0kies
Copy link
Contributor Author

t0kies commented Jul 21, 2023

Yea,

I added the

#elif defined(OLED_TRANSPORT_I2C)
#    include "i2c_master.h"
#    include "keyboard.h"    <-------
#endif

so we don't use the header file with the SPI. Also did a pull request for the edited file.

:D it is working on my side and current device is functioning.

@shadyan
Copy link

shadyan commented Jul 21, 2023

I came here with the exact same issue and just wanted to confirm to that this is indeed a bug/regression, because now you basically can't use an oled and have the two halves use i2c for the matrix, which is common in my experience.

I think you two (@fauxpark and @t0kies) definitely got to the bottom of it as I was able to get my keyboards' firmware to compile by adding #include "keyboard.h" in drivers/oled/oled_driver.c.

Looking forward to the PR being merged. Thank you both!

@drashna
Copy link
Member

drashna commented Sep 30, 2023

Fixed in #21571

@drashna drashna closed this as completed Sep 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants