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

Initial nullbitsco tidbit bring up #424

Merged
merged 1 commit into from Dec 17, 2020

Conversation

mcrosson
Copy link
Contributor

Bring up for the nullbits.co tidbit. This bring up includes 2 keymaps that match the assembly documentation

  • standard "number pad" build
  • "19 keys" build that is more dense

@Nicell Nicell self-assigned this Nov 27, 2020
@Nicell Nicell added enhancement New feature or request shields PRs and issues related to shields labels Nov 27, 2020
@mcrosson mcrosson marked this pull request as ready for review December 2, 2020 04:54
@mcrosson
Copy link
Contributor Author

mcrosson commented Dec 2, 2020

Should I squash this PR's commits ahead of review?

Copy link
Member

@Nicell Nicell left a comment

Choose a reason for hiding this comment

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

Thanks for adding support for this shield. I have a few cursory comments, and then there's a few files that need additions as well.

.github/workflows/build.yml: Add tidbit to the shield list.

docs/docs/hardware.md: You should add the tidbit to this list.

docs/static/setup.ps1 and docs/static/setup.sh: tidbit should be added as a non-split option for both files.

app/boards/shields/tidbit/README.md Outdated Show resolved Hide resolved
app/boards/shields/tidbit/README.md Outdated Show resolved Hide resolved
app/boards/shields/tidbit/README.md Outdated Show resolved Hide resolved
app/boards/shields/tidbit/tidbit.conf Outdated Show resolved Hide resolved
@mcrosson
Copy link
Contributor Author

mcrosson commented Dec 2, 2020

Thanks for adding support for this shield. I have a few cursory comments, and then there's a few files that need additions as well.

.github/workflows/build.yml: Add tidbit to the shield list.

docs/docs/hardware.md: You should add the tidbit to this list.

docs/static/setup.ps1 and docs/static/setup.sh: tidbit should be added as a non-split option for both files.

Added and updates pushed for review

@mcrosson
Copy link
Contributor Author

mcrosson commented Dec 2, 2020

I missed adding the tidbit_19key setup to the build / setup files. The changes/updates should be pushed shortly

@mcrosson
Copy link
Contributor Author

mcrosson commented Dec 2, 2020

I missed adding the tidbit_19key setup to the build / setup files. The changes/updates should be pushed shortly

This may be problematic given the URL and variable assumptions present in the setup scripts. Maybe leave it out for the moment as it looks like some pretty big assumptions made by the setup scripts will break?

@mcrosson mcrosson force-pushed the bringup-tidbit branch 6 times, most recently from 2127a16 to 55b9d5b Compare December 9, 2020 18:24
.github/workflows/build.yml Outdated Show resolved Hide resolved
app/boards/shields/tidbit/README.md Outdated Show resolved Hide resolved
app/boards/shields/tidbit/tidbit.dtsi Outdated Show resolved Hide resolved
docs/static/setup.ps1 Outdated Show resolved Hide resolved
Copy link
Member

@Nicell Nicell left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@mcrosson mcrosson force-pushed the bringup-tidbit branch 2 times, most recently from 35fffe4 to 53f7913 Compare December 14, 2020 18:42
Copy link
Member

@innovaker innovaker left a comment

Choose a reason for hiding this comment

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

Very minor suggestions, but I went the extra mile because you've been giving a lot back to the project.

Should the tidbit_19key be in the setup scripts?

Thanks @mcrosson!

app/boards/shields/tidbit/README.md Outdated Show resolved Hide resolved
app/boards/shields/tidbit/README.md Outdated Show resolved Hide resolved
app/boards/shields/tidbit/README.md Outdated Show resolved Hide resolved
app/boards/shields/tidbit/tidbit.dtsi Outdated Show resolved Hide resolved
app/boards/shields/tidbit/README.md Outdated Show resolved Hide resolved
app/boards/shields/tidbit/tidbit.keymap Outdated Show resolved Hide resolved
app/boards/shields/tidbit/tidbit.keymap Outdated Show resolved Hide resolved
app/boards/shields/tidbit/tidbit_19key.keymap Outdated Show resolved Hide resolved
app/boards/shields/tidbit/tidbit_19key.keymap Outdated Show resolved Hide resolved
docs/docs/hardware.md Outdated Show resolved Hide resolved
@innovaker innovaker assigned mcrosson and unassigned Nicell Dec 16, 2020
@mcrosson
Copy link
Contributor Author

Should the tidbit_19key be in the setup scripts?

It can't be in the setup scripts due to the file path assumptions those scripts have when pulling together the data from the main sources. I have that on my list of things to look at in the future if/when more boards start providing >1 default keymap.

Copy link
Member

@innovaker innovaker left a comment

Choose a reason for hiding this comment

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

Looks good!

app/boards/shields/tidbit/README.md Outdated Show resolved Hide resolved
app/boards/shields/tidbit/README.md Outdated Show resolved Hide resolved
@innovaker innovaker merged commit 565a72b into zmkfirmware:main Dec 17, 2020
1 check passed
@mcrosson mcrosson deleted the bringup-tidbit branch January 23, 2021 19:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request shields PRs and issues related to shields
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants