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

[kle2json] [WIP] populate rotation in info.json #8980

Closed
wants to merge 1 commit into from

Conversation

kbrock
Copy link
Contributor

@kbrock kbrock commented Apr 30, 2020

Overview

QMK uses info.json to know how to render a representation of the keyboard. If the info.json file had rotational data, then QMK could render keyboards that look more similar to the physical keyboard.

Keyboard designers often use keyboard layout editor to design a keyboard layout. From there they want to transfer that information to QMK's info.json file. [kle2json] helps the developers by taking an kle.json file and creating a info.json file.

Part of initiative: qmk/qmk_configurator#725

Types of Changes

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

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

Blocker

Up until this point, the info.json files do not have rotational data.
The configurator could not display it, and the results tended to render better when rotational data was removed.

If the configurator could render keyboards with rotated keys, then it would be great if kle2json could populate those fields. This would help developers better understand the capabilities of the configurator and the QMK suite.

File formats

Both kle.json and info.json have fields to specify the rotation (r, rx, ry).

The future info.json format is setup to also support the addition of those fields.

How to encode rotation information?

Rotate in place

Initially it seemed best to provide an x and y value and rotate the pieces around the center of the piece.

  • converter logic was complicated
  • it made ascii display very difficult
  • it drastically changed the values currently stored in the info files.
  • values in the info file were not obvious how the keyboard worked, and very different from the matrix

Values from kle.json

Next it seemed best to pass the values directly from the kle json file.

  • ascii display required a simple change to add the rotation values with the x and y values.
  • It was still difficult to visualize the wiring matrix
  • It was hard to visualize what it looked like on the screen

Values from kle properties tab

In the end, it worked best to pass the values that kle displays in the properties view.

The values in kle's properties view are different from those in the raw kle file export file. Those values have a more reasonable x and y value so the output makes more sense.

  • converter logic is simple
  • ascii art works without any changes
  • display in the browser is a little more complicated, but very close.
  • calculating the max height and width do need trig functions.
  • values are similar to the current info files
  • layout is more similar to the logical keyboard and wiring than the display on the screen.
  • to follow up on that thought, the current info files are closer to the matrix than the layout.

Conversion

  • kle.json files with no rotational data should generate the same info.json file before and after this change.
  • If possible, kle.json files with rotational data should generate a info.json that is very similar to the non rotational one, just with 3 extra fields populated.
  • fields were rounded to 3 decimal places. The 3rd digit did improve the layout in a noticeable way.

Testing

I used 2 files to test the converter.
I used the [alice layout] and a alternate kle layout that I coded by hand.

From there I rendered the files in the configurator to ensure the results looked as expected.

alternate kle.json

This file was generated by hand, which is not typical.
I used the circuit board to measure the key placement.

[{x:0.45},"PrtScr", {y:0.1,x:0.3},"esc","1",{y:-0.1},"2",{x:8.35},"-",{y:0.1},"+",{w:2},"del"],
[{y:-0.1, x:0.25},"PgUp",{y:0.1,x:0.20,w:1.5},"tab","q",{x:9,y:-0.1},"p",{y:0.1},"[","]",{w:1.5},"\\"],
[{y:-0.1},"PgDwn",{x:0.3,y:0.1,a:7,w:1.75},"Ctrl","a",{x:9.4},";","'",{w:2.25},"enter"],
[{x:1.1, w:2},"shift", "z",{x:9.25},".","?",{w:1.5},"shift","fn"],
[{x:1.1, w:1.5},"code",{x:13.65,w:1.5}, "code"],
[{r:12,rx:4.75,ry:1.15,y:-1},"3","4","5","6"],
[{r:12,rx:4.75,ry:1.15,x:-0.6},"w","e","r","t"],
[{r:12,rx:4.75,ry:1.15,y:1,x:-0.3},"s","d","f","g"],
[{r:12,rx:4.75,ry:1.15,y:2,x:-0.05},"x","c","v", "b"],
[{r:12,rx:4.75,ry:1.15,y:3,x:1.1},{w:2},"sp",{w:1.25},"fn"],
[{r:12,rx:4.75,ry:1.15,y:3.1,x:-0.4},{w:1.5},"alt"],
[{r:-12,rx:13.15,ry:0.975,y:-1,x:-4.05},"7","8","9","0"],
[{r:-12,rx:13.15,ry:0.975,y:0,x:-4.46},"y","u","i","o"],
[{r:-12,rx:13.15,ry:0.975,y:1,x:-4.17},"h","j","k","l"],
[{r:-12,rx:13.15,ry:0.975,y:2,x:-4.48},"b","n","m",","],
[{r:-12,rx:13.15,ry:0.975,y:3,x:-4.48},{w:2.75},"sp"],
[{r:-12,rx:13.15,ry:0.975,y:3,x:-4.48},{x:2.75},{w:1.5, y:0.14},"alt"],

alternateinfo.json

kle2json converted the kle.json to a info.json file that looks very similar before and after the process. There are 3 extra fields populated when the key is rotated.

Also, a bug was fixed with the interaction between x and y values values and their rotational counterparts rx and ry. The introduction of rx and ry reset the position of the keys on the screen.

{
    "keyboard_name": "", 
    "url": "", 
    "maintainer": "qmk", 
    "width": 17.85, 
    "height": 5.25, 
    "layouts": {
        "LAYOUT": {
            "layout": [
                {"label":"PrtScr", "x":0.45, "y":0},
                {"label":"esc", "x":1.75, "y":0.1},
                {"label":"1", "x":2.75, "y":0.1},
                {"label":"2", "x":3.75, "y":0},
                {"label":"-", "x":13.1, "y":0},
                {"label":"+", "x":14.1, "y":0.1},
                {"label":"del", "x":15.1, "y":0.1, "w":2},
                {"label":"PgUp", "x":0.25, "y":1},
                {"label":"tab", "x":1.45, "y":1.1, "w":1.5},
                {"label":"q", "x":2.95, "y":1.1},
                {"label":"p", "x":12.95, "y":1},
                {"label":"[", "x":13.95, "y":1.1},
                {"label":"]", "x":14.95, "y":1.1},
                {"label":"\\", "x":15.95, "y":1.1, "w":1.5},
                {"label":"PgDwn", "x":0, "y":2},
                {"label":"Ctrl", "x":1.3, "y":2.1, "w":1.75},
                {"label":"a", "x":3.05, "y":2.1},
                {"label":";", "x":13.45, "y":2.1},
                {"label":"'", "x":14.45, "y":2.1},
                {"label":"enter", "x":15.45, "y":2.1, "w":2.25},
                {"label":"shift", "x":1.1, "y":3.1, "w":2},
                {"label":"z", "x":3.1, "y":3.1},
                {"label":".", "x":13.35, "y":3.1},
                {"label":"?", "x":14.35, "y":3.1},
                {"label":"shift", "x":15.35, "y":3.1, "w":1.5},
                {"label":"fn", "x":16.85, "y":3.1},
                {"label":"code", "x":1.1, "y":4.1, "w":1.5},
                {"label":"code", "x":16.25, "y":4.1, "w":1.5},
                {"label":"3", "x":4.75, "y":0.15, "r":12, "rx":4.75, "ry":1.15},
                {"label":"4", "x":5.75, "y":0.15, "r":12, "rx":4.75, "ry":1.15},
                {"label":"5", "x":6.75, "y":0.15, "r":12, "rx":4.75, "ry":1.15},
                {"label":"6", "x":7.75, "y":0.15, "r":12, "rx":4.75, "ry":1.15},
                {"label":"w", "x":4.15, "y":1.15, "r":12, "rx":4.75, "ry":1.15},
                {"label":"e", "x":5.15, "y":1.15, "r":12, "rx":4.75, "ry":1.15},
                {"label":"r", "x":6.15, "y":1.15, "r":12, "rx":4.75, "ry":1.15},
                {"label":"t", "x":7.15, "y":1.15, "r":12, "rx":4.75, "ry":1.15},
                {"label":"s", "x":4.45, "y":2.15, "r":12, "rx":4.75, "ry":1.15},
                {"label":"d", "x":5.45, "y":2.15, "r":12, "rx":4.75, "ry":1.15},
                {"label":"f", "x":6.45, "y":2.15, "r":12, "rx":4.75, "ry":1.15},
                {"label":"g", "x":7.45, "y":2.15, "r":12, "rx":4.75, "ry":1.15},
                {"label":"x", "x":4.7, "y":3.15, "r":12, "rx":4.75, "ry":1.15},
                {"label":"c", "x":5.7, "y":3.15, "r":12, "rx":4.75, "ry":1.15},
                {"label":"v", "x":6.7, "y":3.15, "r":12, "rx":4.75, "ry":1.15},
                {"label":"b", "x":7.7, "y":3.15, "r":12, "rx":4.75, "ry":1.15},
                {"label":"sp", "x":5.85, "y":4.15, "w":2, "r":12, "rx":4.75, "ry":1.15},
                {"label":"fn", "x":7.85, "y":4.15, "w":1.25, "r":12, "rx":4.75, "ry":1.15},
                {"label":"alt", "x":4.35, "y":4.25, "w":1.5, "r":12, "rx":4.75, "ry":1.15},
                {"label":"7", "x":9.1, "y":-0.025, "r":-12, "rx":13.15, "ry":0.975},
                {"label":"8", "x":10.1, "y":-0.025, "r":-12, "rx":13.15, "ry":0.975},
                {"label":"9", "x":11.1, "y":-0.025, "r":-12, "rx":13.15, "ry":0.975},
                {"label":"0", "x":12.1, "y":-0.025, "r":-12, "rx":13.15, "ry":0.975},
                {"label":"y", "x":8.69, "y":0.975, "r":-12, "rx":13.15, "ry":0.975},
                {"label":"u", "x":9.69, "y":0.975, "r":-12, "rx":13.15, "ry":0.975},
                {"label":"i", "x":10.69, "y":0.975, "r":-12, "rx":13.15, "ry":0.975},
                {"label":"o", "x":11.69, "y":0.975, "r":-12, "rx":13.15, "ry":0.975},
                {"label":"h", "x":8.98, "y":1.975, "r":-12, "rx":13.15, "ry":0.975},
                {"label":"j", "x":9.98, "y":1.975, "r":-12, "rx":13.15, "ry":0.975},
                {"label":"k", "x":10.98, "y":1.975, "r":-12, "rx":13.15, "ry":0.975},
                {"label":"l", "x":11.98, "y":1.975, "r":-12, "rx":13.15, "ry":0.975},
                {"label":"b", "x":8.67, "y":2.975, "r":-12, "rx":13.15, "ry":0.975},
                {"label":"n", "x":9.67, "y":2.975, "r":-12, "rx":13.15, "ry":0.975},
                {"label":"m", "x":10.67, "y":2.975, "r":-12, "rx":13.15, "ry":0.975},
                {"label":",", "x":11.67, "y":2.975, "r":-12, "rx":13.15, "ry":0.975},
                {"label":"sp", "x":8.67, "y":3.975, "w":2.75, "r":-12, "rx":13.15, "ry":0.975},
                {"label":"alt", "x":11.42, "y":4.115, "w":1.5, "r":-12, "rx":13.15, "ry":0.975}
            ]
        }
    }
}

@kbrock kbrock changed the title [kle2info] populate rotation in info file [kle2json] populate rotation in info file Apr 30, 2020
@zvecr zvecr added cli qmk cli command python labels May 1, 2020
@zvecr zvecr requested review from Erovia, skullydazed and a team May 1, 2020 18:59
@kbrock kbrock changed the title [kle2json] populate rotation in info file [kle2json] populate rotation in info.json May 3, 2020
Copy link
Member

@Erovia Erovia left a comment

Choose a reason for hiding this comment

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

It's a step in the right direction and (seems) backward compatible.

@Erovia Erovia requested review from skullydazed, yanfali and a team June 22, 2020 20:35
@kbrock kbrock changed the title [kle2json] populate rotation in info.json [kle2json] [WIP] populate rotation in info.json Jun 24, 2020
@kbrock
Copy link
Contributor Author

kbrock commented Jun 24, 2020

sorry. wip: need to update the ui to handle these changes

keeping it backwards compatible was simpler. adding the new rotation sub-element adds a little complexity

that extra decimal place looks unnecessary but it is needed
@kbrock
Copy link
Contributor Author

kbrock commented Jul 10, 2020

I removed the changes to the json file format. There are a bunch of proposed changes in that file so going to
stick to the existing format. Will make those changes in a future PR when the format has stabalized.

This format is 100% backwards compatible.
It is also consistent with keyboard layout engine json files.

Looking forward to seeing the info.json evolve, but letting that happen on a different timeline

@stale
Copy link

stale bot commented Aug 24, 2020

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

@stale
Copy link

stale bot commented Sep 23, 2020

Thank you for your contribution!
This pull request has been automatically closed because it has not had activity in the last 30 days. Please feel free to give a status update now, ping for review, or re-open when it's ready.

@stale stale bot closed this Sep 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants