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

Remove invalid pin_compatible config from defaults #19512

Merged
merged 2 commits into from
Jan 7, 2023
Merged

Conversation

zvecr
Copy link
Member

@zvecr zvecr commented Jan 6, 2023

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

@zvecr zvecr requested a review from a team January 6, 2023 00:51
@zvecr zvecr added the bug label Jan 6, 2023
data/mappings/defaults.hjson Outdated Show resolved Hide resolved
data/mappings/defaults.hjson Outdated Show resolved Hide resolved
@sigprof
Copy link
Contributor

sigprof commented Jan 6, 2023

What do you plan to do about Elite-C — update its defaults some time later in develop and fix the fallout there? Having "pin_compatible": "promicro" for Elite-C is wrong in general, but it is not possible to detect automatically that some keyboard has "development_board": "elite_c" only because its maintainer wants the default firmware to match the controller that is commonly sold together with the board. Or should those cases use "development_board": "promicro", "bootloader": "atmel-dfu" instead (which would require #19511 to work)?

(Also it's sad to see the loss of information like #18880 (comment) only because info.json is named as if it actually was JSON.)

The rest of this PR looks correct though.

@zvecr
Copy link
Member Author

zvecr commented Jan 6, 2023

The long term plan is potentially that the elite_c converters can fulfil promicro targets too? Some form of inheritance on the imaginary interfaces they provide. Few issues stopping that at the moment that myself/someone will have to work through.

Short term, I guess i'm not a fan of "development_board": "elite_c" then being used with promicro converters. However as all it does is set bootloader, which the converters will then override, I cant see it doing too much harm if it doesnt use the extra pins.

@zvecr zvecr merged commit f6023a3 into master Jan 7, 2023
@fauxpark fauxpark deleted the zvecr-patch-1 branch January 7, 2023 17:28
omikronik pushed a commit to omikronik/qmk_firmware that referenced this pull request Jan 22, 2023
ideas32 pushed a commit to ideas32/qmk_firmware that referenced this pull request Jan 25, 2023
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.

2 participants