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.

Sign up for GitHub

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

Jump to bottom

Fix QWIIC OLED for AVR #7769

Merged
merged 4 commits into from
Feb 22, 2020
Merged

Fix QWIIC OLED for AVR #7769

merged 4 commits into from
Feb 22, 2020

Conversation

Nicell
Copy link
Contributor

@Nicell Nicell commented Jan 3, 2020 •
edited
Loading

Description

This PR adds support for AVR in the QWIIC OLED driver as well as fixes the buffer sizes by changing their length from width * width / 8 to the proper width * height / 8.

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

@Nicell
Copy link
Contributor Author

Nicell commented Jan 3, 2020

@awkannan can you confirm ARM still works with these changes?

fauxpark
fauxpark reviewed Jan 3, 2020
View reviewed changes
drivers/qwiic/micro_oled.c Outdated Show resolved Hide resolved
@fauxpark fauxpark requested a review from a team January 3, 2020 03:15
@Nicell @fauxpark
Fix typo in comment
1596715 
Co-Authored-By: fauxpark <[email protected]>
drashna
drashna reviewed Jan 3, 2020
View reviewed changes
drivers/qwiic/micro_oled.c
Comment on lines +111 to +115
#ifdef __AVR__
i2c_start(I2C_ADDRESS_SA0_1, 100);
#else
i2c_start(I2C_ADDRESS_SA0_1);
#endif
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a fan of this difference int API.

If anything, the i2c API should be unified, though that should be a separate PR, I think.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for separate PR

@drashna
Copy link
Member

drashna commented Jan 3, 2020

@awkannan and/or @ishtob, your boards are the only ones that use this code. Could one of you check this code change out and verify that it doesn't break your boards?

@Nicell
Copy link
Contributor Author

Nicell commented Jan 20, 2020

Any update on this @drashna? Still awaiting reviews for ARM boards I assume?

@tzarc tzarc mentioned this pull request Feb 21, 2020
Merged
13 tasks
@tzarc
Copy link
Member

tzarc commented Feb 21, 2020

Confirmed working fine on my local ARM board, so long as #8173 gets applied.

@tzarc
Copy link
Member

tzarc commented Feb 21, 2020

While we're fixing this area of the code, can we get micro_oled.h fixed where it checks if LCDWIDTH/LCDHEIGHT aren't set -- it's only checking one of them for both checks:

#ifndef LCDWIDTH
#    define LCDWIDTH 64
#endif
#ifndef LCDWIDTH
#    define LCDHEIGHT 48
#endif

@Nicell
Copy link
Contributor Author

Nicell commented Feb 21, 2020

I've fixed the last incorrect uses of LCDWIDTH where LCDHEIGHT is wanted. Thanks @tzarc for testing this.

@fauxpark
Copy link
Member

Thanks!

@fauxpark fauxpark merged commit d994043 into qmk:master Feb 22, 2020
@Nicell Nicell deleted the qwiic_oled_avr branch February 22, 2020 18:43
sowbug pushed a commit to sowbug/qmk_firmware that referenced this pull request Feb 23, 2020
@Nicell @fauxpark @sowbug
Fix QWIIC OLED for AVR (qmk#7769)
60b9b1e 
* Fix QWIIC OLED for AVR

* Change missed width * width to width * height

* Fix typo in comment

Co-Authored-By: fauxpark <[email protected]>

* Fix last incorrect uses of LCDWIDTH

Co-authored-by: Ryan <[email protected]>
Shinichi-Ohki added a commit to Shinichi-Ohki/qmk_firmware that referenced this pull request Feb 25, 2020
@Shinichi-Ohki
Merge branch 'master' of https://github.com/qmk/qmk_firmware
034c505 
* 'master' of https://github.com/qmk/qmk_firmware: (108 commits)
  `send_unicode_string()`: Add support for code points > 0xFFFF (qmk#8236)
  [Keyboard] Add Wete (qmk#8229)
  Improvements to extrakey HID descriptors (qmk#8156)
  Hineybush h87a lock indicators (qmk#8237)
  Add VIA support for Prime_L (qmk#8233)
  Hub16 - Bug removal + clean up code (qmk#8227)
  [Keyboard] ai03 Equinox (qmk#8224)
  [Keyboard] Add zfrontier/big_switch (qmk#8205)
  Gingham Update (qmk#8225)
  A proper `send_string()` for the Unicode feature (qmk#8155)
  Rollback PR qmk#7967 in preference of fixing I2C start/stop properly, in a followup PR. (qmk#8173)
  Add mouse support to SEND_STRING (qmk#8223)
  Add link to "Useful functions" in macro docs (qmk#7446)
  New functionality for cformat (qmk#7893)
  Update main.c (qmk#8198)
  format code according to conventions [skip ci]
  Fix QWIIC OLED for AVR (qmk#7769)
  VIA Support: KBD75 rev1/rev2 (qmk#8214)
  Update TMOv2 for new key (qmk#7759)
  Added custom keymap for preonic (qmk#7548)
  ...
HokieGeek pushed a commit to HokieGeek/qmk_firmware that referenced this pull request Mar 5, 2020
@Nicell @fauxpark @HokieGeek
Fix QWIIC OLED for AVR (qmk#7769)
8f38658 
* Fix QWIIC OLED for AVR

* Change missed width * width to width * height

* Fix typo in comment

Co-Authored-By: fauxpark <[email protected]>

* Fix last incorrect uses of LCDWIDTH

Co-authored-by: Ryan <[email protected]>
c0psrul3 pushed a commit to c0psrul3/qmk_firmware that referenced this pull request Mar 23, 2020
@Nicell @fauxpark @c0psrul3
Fix QWIIC OLED for AVR (qmk#7769)
a7eaf0d 
* Fix QWIIC OLED for AVR

* Change missed width * width to width * height

* Fix typo in comment

Co-Authored-By: fauxpark <[email protected]>

* Fix last incorrect uses of LCDWIDTH

Co-authored-by: Ryan <[email protected]>
drashna pushed a commit to zsa/qmk_firmware that referenced this pull request Mar 26, 2020
@Nicell @drashna
Fix QWIIC OLED for AVR (qmk#7769)
177f561 
* Fix QWIIC OLED for AVR

* Change missed width * width to width * height

* Fix typo in comment

Co-Authored-By: fauxpark <[email protected]>

* Fix last incorrect uses of LCDWIDTH

Co-authored-by: Ryan <[email protected]>

format code according to conventions [skip ci]
kylekuj pushed a commit to kylekuj/qmk_firmware that referenced this pull request Apr 21, 2020
@Nicell @fauxpark @kujawakk
Fix QWIIC OLED for AVR (qmk#7769)
8481874 
* Fix QWIIC OLED for AVR

* Change missed width * width to width * height

* Fix typo in comment

Co-Authored-By: fauxpark <[email protected]>

* Fix last incorrect uses of LCDWIDTH

Co-authored-by: Ryan <[email protected]>
jakeisnt pushed a commit to jakeisnt/qmk_firmware that referenced this pull request Aug 20, 2020
@Nicell @fauxpark @jakeisnt
Fix QWIIC OLED for AVR (qmk#7769)
6d16814 
* Fix QWIIC OLED for AVR

* Change missed width * width to width * height

* Fix typo in comment

Co-Authored-By: fauxpark <[email protected]>

* Fix last incorrect uses of LCDWIDTH

Co-authored-by: Ryan <[email protected]>
BorisTestov pushed a commit to BorisTestov/qmk_firmware that referenced this pull request May 23, 2024
@Nicell @fauxpark
Fix QWIIC OLED for AVR (qmk#7769)
ccdd667 
* Fix QWIIC OLED for AVR

* Change missed width * width to width * height

* Fix typo in comment

Co-Authored-By: fauxpark <[email protected]>

* Fix last incorrect uses of LCDWIDTH

Co-authored-by: Ryan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zvecr zvecr zvecr approved these changes

@fauxpark fauxpark fauxpark approved these changes

@drashna drashna drashna approved these changes

Assignees
No one assigned
Labels
bug core
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Nicell @drashna @tzarc @fauxpark @zvecr