Skip to content

Pimoroni DV Base: Fix i2s audio pin assignments#8906

Merged
tannewt merged 3 commits intoadafruit:8.2.xfrom
RetiredWizard:pimoroni_i2sfix
Feb 13, 2024
Merged

Pimoroni DV Base: Fix i2s audio pin assignments#8906
tannewt merged 3 commits intoadafruit:8.2.xfrom
RetiredWizard:pimoroni_i2sfix

Conversation

@RetiredWizard
Copy link
Copy Markdown

@RetiredWizard RetiredWizard commented Feb 12, 2024

I think I've set this up against the 8.2.x branch, let me know if I've done it wrong.

The same changes will be needed for the Pico W DV base but that didn't get introduced until 9.x so I can make a second PR with both this change and the change for the second board against the main branch, if that makes sense.

Thanks!

@RetiredWizard
Copy link
Copy Markdown
Author

RetiredWizard commented Feb 12, 2024

When I run pre-commit locally there are 71 files in shared-bindings, tests and tools that appear to have minor formatting updates. Should I go ahead and sync those changes?

@jepler
Copy link
Copy Markdown

jepler commented Feb 12, 2024

It's odd that there are pre-commit changes in files you didn't modify. Someone will need to determine why these changes are being made by pre-commit now.

@RetiredWizard
Copy link
Copy Markdown
Author

RetiredWizard commented Feb 12, 2024 •
edited
Loading

It's odd that there are pre-commit changes in files you didn't modify. Someone will need to determine why these changes are being made by pre-commit now.

If it helps, I cloned the repo against main instead of 8.2.x and before making any changes ran pre-commit and also got the same 70 formatting changes.

This is the first time I'm building/creating a PR after re-installing my OS and remaking the build environment, but I don't seem to be having any issues creating working UF2 images.

@tannewt
Copy link
Copy Markdown
Member

tannewt commented Feb 12, 2024

I suspect that the black version change to black==24.1.1 from black==23.12.1 caused these changes. Looks like they now have an annual "stable" style: https://github.com/psf/black/releases/tag/24.1.0

I think it's ok to include all of the updates to this PR. The CI will fail without them.

@jepler
Copy link
Copy Markdown

jepler commented Feb 12, 2024

Oh, and we aren't pinning black? Makes sense then, thanks for checking into it

@RetiredWizard
Copy link
Copy Markdown
Author

RetiredWizard commented Feb 12, 2024

Well that was a pain, graphical VSCODE UI didn't want to commit the pre-commit changes, I ended up running the command line commit process... Hopefully I got it right 😁

@RetiredWizard
Copy link
Copy Markdown
Author

RetiredWizard commented Feb 12, 2024

The DOCS build check failed 😦

The log warnings I spotted:

circuitpython-stubs/memorymap/__init__.pyi
[WARN] Missing argument type: start on line 26
[WARN] Missing argument type: length on line 26

circuitpython-stubs/synthio/__init__.pyi
[WARN] Missing return type: __init__ on line 129
[WARN] Missing return type: retrigger on line 166
[WARN] Missing return type: __init__ on line 234
[WARN] Missing argument type: once on line 137
[WARN] Missing argument type: interpolate on line 138
[WARN] Missing argument type: retrigger on line 429

circuitpython-stubs/ulab/numpy/carray/__init__.pyi
[WARN] Missing return type: real on line 2
[WARN] Missing return type: imag on line 8
[WARN] Missing return type: conjugate on line 14
[WARN] Missing argument type: val on line 2
[WARN] Missing argument type: val on line 8
[WARN] Missing argument type: val on line 14

circuitpython-stubs/espulp/__init__.pyi
[WARN] Missing return type: __init__ on line 38

WARNING setuptools_scm.pyproject_reading toml section missing 'pyproject.toml does not contain a tool.setuptools_scm section'
/tmp/build-env-0jn42jbh/lib/python3.12/site-packages/setuptools_scm/git.py:163: UserWarning: "/home/runner/work/circuitpython/circuitpython" is shallow and may cause errors
  warnings.warn(f'"{wd.path}" is shallow and may cause errors')

WARNING setuptools_scm.pyproject_reading toml section missing 'pyproject.toml does not contain a tool.setuptools_scm section'

ERROR setuptools_scm._file_finders.git listing git files failed - pretending there aren't any

Successfully built circuitpython-stubs-8.2.10.dev9.tar.gz and circuitpython_stubs-8.2.10.dev9-py3-none-any.whl
synthio/__init__.pyi:138: error: Incompatible default for argument "waveform" (default has type "None", argument has type "array[Any] | bytearray | bytes | memoryview | RGBMatrix | ndarray")  [assignment]
synthio/__init__.pyi:138: note: PEP 484 prohibits implicit Optional. Accordingly, mypy has changed its default to no_implicit_optional=True
synthio/__init__.pyi:138: note: Use https://github.com/hauntsaninja/no_implicit_optional to automatically upgrade your codebase
synthio/__init__.pyi:331: error: Incompatible default for argument "ring_waveform" (default has type "float", argument has type "array[Any] | bytearray | bytes | memoryview | RGBMatrix | ndarray | None")  [assignment]
Found 2 errors in 1 file (checked 95 source files)
make: *** [Makefile:274: check-stubs] Error 1
Error: Process completed with exit code 2.

@tannewt
Copy link
Copy Markdown
Member

tannewt commented Feb 13, 2024

You need these two fixes: https://github.com/adafruit/circuitpython/pull/8895/files

@tannewt tannewt mentioned this pull request Feb 13, 2024
Merged
@RetiredWizard
Copy link
Copy Markdown
Author

RetiredWizard commented Feb 13, 2024

Thanks @tannewt ignore my link, I was trying to reference the PR you mentioned but picked up a micropython PR instead 😦

tannewt
tannewt approved these changes Feb 13, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Thank you!

@tannewt tannewt merged commit 722f75f into adafruit:8.2.x Feb 13, 2024
@RetiredWizard RetiredWizard deleted the pimoroni_i2sfix branch February 13, 2024 18:37
@RetiredWizard RetiredWizard mentioned this pull request Feb 14, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tannewt tannewt tannewt approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@RetiredWizard @jepler @tannewt