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

Fixes non-unique vid and pid for sofle_choc #23080

Closed
wants to merge 0 commits into from

Conversation

JellyTitan
Copy link
Contributor

The sofle_choc has the same VendorID and ProductID as sofle/rev1. This PR changes the VID/PID for sofle_choc to make them unique. The keyboard creator, Brian Low, approved the VID/PID update.
I checked for conflicts between the new VID/PID in the latest QMK and didn't find any.

I went with "BL" (Brian Low) for vendor ID & "SC" (Sofle Choc) for ProductID.

@brianlow tagging for visibility.

Description

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: C, Python
  • I have read the PR Checklist document and have made the appropriate changes.
  • 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).

@t-8ch
Copy link
Contributor

t-8ch commented Feb 24, 2024

Wouldn't it be better to allocate a pid from https://pid.codes instead as that does not risk conflicts?

@JellyTitan
Copy link
Contributor Author

@t-8ch

Wouldn't it be better to allocate a pid from https://pid.codes instead as that does not risk conflicts?

Is there any official QMK guidance on this?

It looks like many devices in QMK are built/maintained by individuals, and the pid.codes registry seems geared towards organizations.

If I'm adding VID/PID for other peoples projects, then I'd need to create org profiles for other people - which feels weird?

@keyboard-magpie
Copy link
Contributor

https://docs.qmk.fm/#/faq_build?id=usb-vid-and-pid

I don't understand why sofle choc needs a different pair than the original sofle though? They have the same key layout etc.

@JellyTitan
Copy link
Contributor Author

https://docs.qmk.fm/#/faq_build?id=usb-vid-and-pid

I don't understand why sofle choc needs a different pair than the original sofle though? They have the same key layout etc.

The Sofle Choc needs a unique pair because it has RGB, and the original Sofle does not.

@keyboard-magpie
Copy link
Contributor

There are plenty non-choc sofles with RGB. It would make more sense to me (presuming this is for VIA purposes) to edit the VIA json.

@JellyTitan
Copy link
Contributor Author

There are plenty non-choc sofles with RGB. It would make more sense to me (presuming this is for VIA purposes) to edit the VIA json.

There is one Sofle RGB already in VIA -but that is the keyhive variant, which has a different pinout, so that VIA config is not compatible with the Sofle Choc.

The keyhive unique pinout variant issue is mentioned here.

This PR is for VIA purposes. The VIA PR is blocked by the Sofle Choc having a non-unique PID/VID. The VIA issue aside, the Sofle Choc should still have a unique PID/VID.

@JellyTitan
Copy link
Contributor Author

@t-8ch

Wouldn't it be better to allocate a pid from https://pid.codes instead as that does not risk conflicts?

I followed up on this in the QMK discord channel. Using pid.codes is not a requirement, and I would feel weird creating profiles for other people, so I will not update this PR to use pid.codes.

The initial pull request PID/VID remains unchanged.

@t-8ch
Copy link
Contributor

t-8ch commented Mar 11, 2024

@JellyTitan

The initial pull request PID/VID remains unchanged.

Ok

Copy link

github-actions bot commented May 6, 2024

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 bug, awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

@github-actions github-actions bot added the stale Issues or pull requests that have become inactive without resolution. label May 6, 2024
@JellyTitan
Copy link
Contributor Author

All issues have been addressed. Waiting for review.

@github-actions github-actions bot removed the stale Issues or pull requests that have become inactive without resolution. label May 8, 2024
@JellyTitan
Copy link
Contributor Author

@drashna would you be so kind as to review? The PR has been approved by the boards creator, and all other concerns have been addressed. (Plus it's only two lines)!

Copy link

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 bug, awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

@github-actions github-actions bot added the stale Issues or pull requests that have become inactive without resolution. label Jun 29, 2024
@JellyTitan JellyTitan closed this Jul 1, 2024
@JellyTitan JellyTitan force-pushed the sofle_choc_vid_fix branch from 1a822f8 to 3ffe8d9 Compare July 1, 2024 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keyboard stale Issues or pull requests that have become inactive without resolution.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants