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

Bugfix for quantum/dip_switch.c #8731

Merged
merged 6 commits into from
Apr 12, 2020
Merged

Conversation

mtei
Copy link
Contributor

@mtei mtei commented Apr 8, 2020

Description

memcpy(last_dip_switch_state, dip_switch_state, sizeof(&dip_switch_state));

The value of sizeof(&dip_switch_state) is always set to 2. It should be sizeof(dip_switch_state).

Types of Changes

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

Issues Fixed or Closed by This PR

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.
  • 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).

@mtei mtei requested review from drashna and a team April 8, 2020 10:12
@zvecr zvecr requested review from jackhumbert and a team April 8, 2020 20:27
quantum/dip_switch.c Outdated Show resolved Hide resolved
@mtei
Copy link
Contributor Author

mtei commented Apr 11, 2020

@drashna , could you please confirm this PR?

Copy link
Member

@drashna drashna left a comment

Choose a reason for hiding this comment

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

Honestly, most of this code was copy-pasted from the planck custom matrix...

However, tested the change on my Planck rev6 and works as expected.

@drashna drashna changed the title Bugfix quantum/dip_switch.c Bugfix for quantum/dip_switch.c Apr 12, 2020
@drashna drashna merged commit d3c29c9 into qmk:master Apr 12, 2020
@drashna
Copy link
Member

drashna commented Apr 12, 2020

And nice catch!

@mtei
Copy link
Contributor Author

mtei commented Apr 12, 2020

FYI:

This bug causes undesirable behavior when there are three or more dip switches (five or more in the case of ARM). The callback function is called on every scan, as shown in the following example.

*** Yushakobo - Helix Beta disconnected -- 0xFEED:0x3060
*** Yushakobo - Helix Beta connected -- 0xFEED:0x3060
  >  002001 : dump_pbuf 16
     000000 : keyboard_pre_init_user()
     000000 : dip_switch_update_user(0,1)
  >  000000 : dip_switch_update_user(1,1)
     000000 : dip_switch_update_user(2,1)
     000000 : dip_switch_update_user(3,1)
     000000 : dip_switch_update_mask_user(0b1111)
  >  000000 : keyboard_post_init_user()
     000001 : dip_switch_update_user(2,1)
     000001 : dip_switch_update_user(3,1)
     000001 : dip_switch_update_mask_user(0b1111)
     000001 : dip_switch_update_user(2,1)
     000001 : dip_switch_update_user(3,1)
     000001 : dip_switch_update_mask_user(0b1111)
     000002 : dip_switch_update_user(2,1)
     000002 : dip_switch_update_user(3,1)
     000002 : dip_switch_update_mask_user(0b1111)

@drashna
Copy link
Member

drashna commented Apr 12, 2020

I've seen reports of a slowed down matrix scanning since the code was moved into core. If it's calling the called on every scan, that definitely could be the cause of the issue.

calmh added a commit to calmh/qmk_firmware that referenced this pull request Apr 13, 2020
* master: (973 commits)
  Fix AVR SPI parameter configuration, remove timeouts due to sync protocol. (qmk#8775)
  VIA Support: Jane V2 (qmk#8735)
  Add a simple custom keymap for Gergo. (qmk#8662)
  Add via support to keebio/bdn9 (qmk#8620)
  DP60 VIA cleanups (qmk#8697)
  Adding Niu Mini to VIA (qmk#8702)
  Allow trailing whitespace in markdown docs, for formatting purposes. (qmk#8774)
  Add support for hardware and board initialisation overrides. (qmk#8330)
  [Keyboard] Add IDOBAO ID80 (qmk#8728)
  [Keyboard] Quefrency Rev2 Caps Lock LED, set lighting defaults (qmk#8729)
  [Keyboard] Add handwired Fc200rt qmk board (qmk#8726)
  Bugfix for quantum/dip_switch.c (qmk#8731)
  Add *OPT aliases for *ALT keycodes and macros (qmk#8714)
  [Keymap] Add keymap for Nyquist rev3 (qmk#8706)
  format code according to conventions [skip ci]
  Added Workman ZXCVM variation (qmk#8686)
  [Keyboard] jotpad16 status leds (qmk#8643)
  [Keyboard] Add handwired BDN9-BLE (qmk#8192)
  Fix bug with layer caching in get_event_keycode (qmk#8693)
  [Keyboard] Add CannonKeys Atlas keyboard (qmk#8207)
  ...
Quarren42 pushed a commit to Quarren42/qmk_firmware that referenced this pull request Apr 15, 2020
* dipsw test on helix/rev2/sc/back:five_rows

* bug fix quantum/dip_switch.c

* test end. remove test code. Revert "dipsw test on helix/rev2/sc/back:five_rows"

This reverts commit 4b13ebb.

* dipsw test on helix/rev2/sc/back:five_rows

* update quantum/dip_switch.c

* test end. remove test code. Revert "dipsw test on helix/rev2/sc/back:five_rows"

This reverts commit bf99ace.
@mtei mtei deleted the bugfix__dip_switch.c branch April 21, 2020 12:55
kylekuj pushed a commit to kylekuj/qmk_firmware that referenced this pull request Apr 21, 2020
* dipsw test on helix/rev2/sc/back:five_rows

* bug fix quantum/dip_switch.c

* test end. remove test code. Revert "dipsw test on helix/rev2/sc/back:five_rows"

This reverts commit 4b13ebb.

* dipsw test on helix/rev2/sc/back:five_rows

* update quantum/dip_switch.c

* test end. remove test code. Revert "dipsw test on helix/rev2/sc/back:five_rows"

This reverts commit bf99ace.
violet-fish pushed a commit to violet-fish/qmk_firmware that referenced this pull request May 3, 2020
* dipsw test on helix/rev2/sc/back:five_rows

* bug fix quantum/dip_switch.c

* test end. remove test code. Revert "dipsw test on helix/rev2/sc/back:five_rows"

This reverts commit 4b13ebb.

* dipsw test on helix/rev2/sc/back:five_rows

* update quantum/dip_switch.c

* test end. remove test code. Revert "dipsw test on helix/rev2/sc/back:five_rows"

This reverts commit bf99ace.
bitherder pushed a commit to bitherder/qmk_firmware that referenced this pull request May 15, 2020
* dipsw test on helix/rev2/sc/back:five_rows

* bug fix quantum/dip_switch.c

* test end. remove test code. Revert "dipsw test on helix/rev2/sc/back:five_rows"

This reverts commit 4b13ebb.

* dipsw test on helix/rev2/sc/back:five_rows

* update quantum/dip_switch.c

* test end. remove test code. Revert "dipsw test on helix/rev2/sc/back:five_rows"

This reverts commit bf99ace.
drashna pushed a commit to zsa/qmk_firmware that referenced this pull request May 24, 2020
* dipsw test on helix/rev2/sc/back:five_rows

* bug fix quantum/dip_switch.c

* test end. remove test code. Revert "dipsw test on helix/rev2/sc/back:five_rows"

This reverts commit 4b13ebb.

* dipsw test on helix/rev2/sc/back:five_rows

* update quantum/dip_switch.c

* test end. remove test code. Revert "dipsw test on helix/rev2/sc/back:five_rows"

This reverts commit bf99ace.
sowbug pushed a commit to sowbug/qmk_firmware that referenced this pull request May 24, 2020
* dipsw test on helix/rev2/sc/back:five_rows

* bug fix quantum/dip_switch.c

* test end. remove test code. Revert "dipsw test on helix/rev2/sc/back:five_rows"

This reverts commit 4b13ebb.

* dipsw test on helix/rev2/sc/back:five_rows

* update quantum/dip_switch.c

* test end. remove test code. Revert "dipsw test on helix/rev2/sc/back:five_rows"

This reverts commit bf99ace.
fdidron pushed a commit to zsa/qmk_firmware that referenced this pull request Jun 12, 2020
* dipsw test on helix/rev2/sc/back:five_rows

* bug fix quantum/dip_switch.c

* test end. remove test code. Revert "dipsw test on helix/rev2/sc/back:five_rows"

This reverts commit 4b13ebb.

* dipsw test on helix/rev2/sc/back:five_rows

* update quantum/dip_switch.c

* test end. remove test code. Revert "dipsw test on helix/rev2/sc/back:five_rows"

This reverts commit bf99ace.
turky pushed a commit to turky/qmk_firmware that referenced this pull request Jun 13, 2020
* dipsw test on helix/rev2/sc/back:five_rows

* bug fix quantum/dip_switch.c

* test end. remove test code. Revert "dipsw test on helix/rev2/sc/back:five_rows"

This reverts commit 4b13ebb.

* dipsw test on helix/rev2/sc/back:five_rows

* update quantum/dip_switch.c

* test end. remove test code. Revert "dipsw test on helix/rev2/sc/back:five_rows"

This reverts commit bf99ace.
jakobaa pushed a commit to jakobaa/qmk_firmware that referenced this pull request Jul 7, 2020
* dipsw test on helix/rev2/sc/back:five_rows

* bug fix quantum/dip_switch.c

* test end. remove test code. Revert "dipsw test on helix/rev2/sc/back:five_rows"

This reverts commit 4b13ebb.

* dipsw test on helix/rev2/sc/back:five_rows

* update quantum/dip_switch.c

* test end. remove test code. Revert "dipsw test on helix/rev2/sc/back:five_rows"

This reverts commit bf99ace.
BorisTestov pushed a commit to BorisTestov/qmk_firmware that referenced this pull request May 23, 2024
* dipsw test on helix/rev2/sc/back:five_rows

* bug fix quantum/dip_switch.c

* test end. remove test code. Revert "dipsw test on helix/rev2/sc/back:five_rows"

This reverts commit 4b13ebb.

* dipsw test on helix/rev2/sc/back:five_rows

* update quantum/dip_switch.c

* test end. remove test code. Revert "dipsw test on helix/rev2/sc/back:five_rows"

This reverts commit bf99ace.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants