kiibohd / controller

Kiibohd Controller
GNU General Public License v3.0
807 stars 270 forks source link

Merged the latest changes from this past weekend into my fork - the compiled software is really buggy. #180

Closed JBonifacio closed 7 years ago

JBonifacio commented 7 years ago

I merged commits a1a99cf014be160db9dda2b62cd7c7ee65d690ed and 2c3e555acfe81bfa96e18c6c4604ba6e58d160ec from the past weekend (01/29/17) into my forked repo and tried compiling my firmware. Compiling didn't report any errors, so I flashed the firmware onto my Whitefox. The firmware had a few odd problems.

I've reverted to my commit before merging in. This resolved the issue when I recompiled my firmware and installed it on my Whitefox.

AndyBette commented 7 years ago

Had similar problems with just the left side of my ergodox: NKRO stopped working and the GUI key doesn't register sometimes, and other times repeated uncontrollably. The uncontrolled signals from my board brought down MacOS. Haven't had time to chase down the bug yet but will comment if I find anything.

haata commented 7 years ago

Hmm, I've been working on the capabilities a lot.

Make sure you've updated the kll compiler as well.

On Tue, Jan 31, 2017, 12:49 Andy Bette notifications@github.com wrote:

Had similar problems with just the left side of my ergodox: NKRO stopped working and the GUI key doesn't register sometimes, and other times start repeating uncontrollably. The uncontrolled signals from my board brought down MacOS. Haven't had time to chase down the bug yet but will comment if I find anything.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/kiibohd/controller/issues/180#issuecomment-276487460, or mute the thread https://github.com/notifications/unsubscribe-auth/AABbqQikRWnpG3yVYxuu5DJ6nrPjnxKDks5rX55BgaJpZM4LzIfd .

JBonifacio commented 7 years ago

I fetched and merged both the kiibohd/kll and kiibohd/controller into my forks... I even tried cloning the repo from scratch. I still ran into the issue.

AndyBette commented 7 years ago

I rolled both the controller and KLL back to their state at the beginning of the year and didn't have the issue. Definitely due to one of the recent commits but I'm new to this kind of hardware so I may not be much help with debugging. Gut feeling is that it has something to do with the addition of the TriggerMacro parameter.

haata commented 7 years ago

Yeah, the problem is likely the new parameter. Though in the master branch I don't actually use it for anything (at least on purpose anyways).

The easiest way to debug is using the virtual serial port cli and print statements. (especially because the keyboard does seem functional'ish).

https://github.com/kiibohd/controller/wiki/Debugging

This change was added so I can support per-key relative animations on the K-Type. Once it's all ready, I'll be porting it back to Ergodox and Whitefox as well. (see the ktype branch, though right now Whitefox and Ergodox don't compile yet).

AndyBette commented 7 years ago

Confirmed, the commits from the past two days introduced the problem

haata commented 7 years ago

Also, can both of you link your GitHub forks so I can see what changes you've made?

On Tue, Jan 31, 2017, 17:48 Andy Bette notifications@github.com wrote:

Confirmed, the commits from the past two days introduced the problem

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/kiibohd/controller/issues/180#issuecomment-276550729, or mute the thread https://github.com/notifications/unsubscribe-auth/AABbqe5BseKJWI3XfH7EUK6HGFb6geLvks5rX-SGgaJpZM4LzIfd .

AndyBette commented 7 years ago

no changes, I just branched from these commits: https://github.com/kiibohd/kll/commit/76f50116981df96101e0b3ac91e51c6101b91bca and https://github.com/kiibohd/controller/commit/ea3be696209ee2179e21313bd76b166e0a0998f6 then compiled.

JBonifacio commented 7 years ago

https://github.com/JBonifacio/kll and https://github.com/JBonifacio/controller, if this information helps.

haata commented 7 years ago

While I debug this, for those that are interested. These are the offending commits:

haata commented 7 years ago

Hmm, looks like these aren't the offending commits. Something else more sinister is going on.

It seems I'm not filtering multiple key-presses on a single scan loop. (I've never had to because the scan loops are so fast, but how I'm doing the scanning has changed)

I have some ideas on how to fix this.

AndyBette commented 7 years ago

Did you change the scanning method between Friday and now? I don't have any issues while using the firmware I compiled by branching from the commits I mentioned above (basically the state of the repos from late last week).

haata commented 7 years ago

No, at least not on master anyways. I think this issue may have always been there, but the additional overhead of adding more parameters may have pushed it over the edge.

The 6KRO modifier bug is related to this I think.

haata commented 7 years ago

Aha! I found it.

https://github.com/kiibohd/controller/blob/master/Macro/PartialMap/result.c#L142

Should be

switch( Macro_evalResultMacro( macroResultMacroPendingList.data[ macro ] ) )

I'm not going to push this just yet, soon though. I found a bunch of other issues that I'm going to test altogether.

haata commented 7 years ago

Should be fixed now. Thanks everyone for helping out.

Let me know if there are any other issues with master.