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.

Sign up for GitHub

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

Jump to bottom

Westberrytech pr #14422

Merged
merged 4 commits into from Nov 26, 2021
Merged

Westberrytech pr #14422

merged 4 commits into from Nov 26, 2021

Conversation

itarze
Copy link
Contributor

@itarze itarze commented Sep 13, 2021 •
edited

Description

Added WB32F3G71xx support

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

@drashna drashna requested a review from a team September 13, 2021 20:16
@drashna
Copy link
Member

drashna commented Sep 14, 2021

This needs to be split up into several PRs. At the very least, the keyboard code needs to be pulled out into a separate PR, so this is just adding the platform files for the WB32 controller.

@itarze
Copy link
Contributor Author

itarze commented Sep 14, 2021

@drashna I have deleted the keyboard code , Wait for this pr to pass before submitting the keyboard code.

drashna
drashna requested changes Sep 14, 2021
View reviewed changes
bootloader.mk Outdated Show resolved Hide resolved
platforms/chibios/drivers/i2c_master.c Outdated Show resolved Hide resolved
platforms/chibios/flash.mk Outdated Show resolved Hide resolved
drashna
drashna previously approved these changes Oct 23, 2021
View reviewed changes
@drashna drashna requested a review from a team October 23, 2021 06:23
@itarze
Copy link
Contributor Author

itarze commented Oct 27, 2021

@fauxpark Hi, can you through this PR?

itarze
itarze commented Nov 1, 2021
View reviewed changes
Copy link
Contributor Author

@itarze itarze left a comment

Choose a reason for hiding this comment

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

pass

@drashna drashna requested a review from tzarc November 10, 2021 15:19
@drashna
Copy link
Member

drashna commented Nov 11, 2021

should be fine to ignore this specific error, IIRC.

@itarze
Copy link
Contributor Author

itarze commented Nov 11, 2021

@drashna May I ask when this PR can be merged or is there anything else that needs to be fixed? Our company is in a hurry.

@drashna
Copy link
Member

drashna commented Nov 11, 2021

It's waiting on another approval, as we generally only merge PRs after 2 approvals, especially for core changes.

However, the hope is that this gets into this breaking changes cycle, so that it's merged into master, at the end of the month.

@itarze
Copy link
Contributor Author

itarze commented Nov 11, 2021

@drashna Can you help me to urge the another approver? He never replied to me. Please

zvecr
zvecr requested changes Nov 13, 2021
View reviewed changes
platforms/chibios/drivers/uart.c Outdated Show resolved Hide resolved
platforms/chibios/drivers/spi_master.c Outdated Show resolved Hide resolved
platforms/chibios/drivers/i2c_master.c Outdated Show resolved Hide resolved
zvecr
zvecr requested changes Nov 13, 2021
View reviewed changes
lib/python/qmk/cli/doctor/linux.py Outdated Show resolved Hide resolved
platforms/chibios/drivers/spi_master.c Outdated Show resolved Hide resolved
tmk_core/common/chibios/bootloader.c Outdated Show resolved Hide resolved
tmk_core/common/chibios/eeprom_wb32.c Outdated Show resolved Hide resolved
itarze
itarze commented Nov 18, 2021
View reviewed changes
Copy link
Contributor Author

@itarze itarze left a comment

Choose a reason for hiding this comment

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

WB32 flash has 200k+ write cycle endurance.
I have modified the code, please check whether there are other problems.

zvecr
zvecr reviewed Nov 18, 2021
View reviewed changes
tmk_core/common/chibios/eeprom_wb32.c Outdated

WriteLen = (Len < PageReamin) ? Len : PageReamin;

if (WriteLen != FEE_PAGE_SIZE) {
Copy link
Member

@zvecr zvecr Nov 18, 2021 •
edited

Choose a reason for hiding this comment

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

If a byte is already written,

The function docs still does not match the behaviour. All that has been added is an optimisation when writes align to page size, along with a load of duplication and a dead return path.

@itarze
Copy link
Contributor Author

itarze commented Nov 19, 2021

I'll remove the code related to the eeprom_wb32 first.
Our customers can use external eeprom to store data.

@itarze itarze requested review from tzarc and zvecr November 19, 2021 01:47
@peepeetee
Copy link
Contributor

Is there any language help I can offer here? I could be reached at peepeetee@protonmail.com
在？我来帮您说英语？在peepeetee@protonmail.com联系我

@drashna drashna self-requested a review November 19, 2021 06:11
@drashna
Copy link
Member

drashna commented Nov 19, 2021

This has some merge conflicts that need to be resolved.

@itarze
Copy link
Contributor Author

itarze commented Nov 19, 2021

This has some merge conflicts that need to be resolved.

I have solved it, please review it again.

@itarze
Copy link
Contributor Author

itarze commented Nov 19, 2021

Is it ok to pass now?
Two reviewers have been approved.

@tzarc
Copy link
Member

tzarc commented Nov 19, 2021 •
edited

Is it ok to pass now? Two reviewers have been approved.

There's only one green tick.

image

@drashna drashna dismissed their stale review November 19, 2021 21:06

defer to experts

@itarze

This comment has been minimized.

Sign in to view
@tzarc
Copy link
Member

tzarc commented Nov 19, 2021

That was old and has since been rescinded.

image

@itarze
Copy link
Contributor Author

itarze commented Nov 19, 2021

Okay, so now I just need to wait other reviewer approval ？

@tzarc
Copy link
Member

tzarc commented Nov 19, 2021

Yes.

@itarze
Copy link
Contributor Author

itarze commented Nov 19, 2021

Thanks ！

@tzarc tzarc merged commit 68838bb into qmk:develop Nov 26, 2021
@itarze itarze deleted the westberrytech_pr branch December 3, 2021 05:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@peepeetee peepeetee peepeetee left review comments

@tzarc tzarc tzarc approved these changes

@zvecr zvecr zvecr approved these changes

@fauxpark fauxpark Awaiting requested review from fauxpark

@drashna drashna Awaiting requested review from drashna

Assignees
No one assigned
Labels
cli qmk cli command core documentation python
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@itarze @drashna @peepeetee @tzarc @fauxpark @zvecr