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

Fix spelling #998

Merged
merged 4 commits into from
Sep 13, 2024
Merged

Fix spelling #998

merged 4 commits into from
Sep 13, 2024

Conversation

2bndy5
Copy link
Member

@2bndy5 2bndy5 commented Sep 8, 2024

Add spell checking

Locally, I've been using this cspell VSCode extension (for a while), but I think it would be better if the remote adhered to a lesser quantity of spelling errors.

In the bigger picture, I think this would help those using google to translate our English docs into their native tongue. For day-to-day development, this should prevent the numerous copied-and-pasted spelling errors (in examples and driver code)

I added a CI job to check spelling (which also uses cSpell) in all source files including

  • cmake scripts (I might ignore these in the future)
  • C/C++ sources (except BCM2835 lib sources)
  • *.md files (& *.rst files for sphinx docs)
  • *.py files

Note

Spell checking is not applied to the configure script nor any Makefiles.

Comment on lines -288 to +289
.def(bp::init<uint16_t, uint16_t, uint32_t>((bp::arg("_cepin"), bp::arg("_cspin"), bp::arg("spispeed"))))
.def(bp::init<uint32_t>((bp::arg("spispeed"))))
.def(bp::init<uint16_t, uint16_t, uint32_t>((bp::arg("_cepin"), bp::arg("_cspin"), bp::arg("spi_speed"))))
.def(bp::init<uint32_t>((bp::arg("spi_speed"))))
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the only questionable change because it could break existing user code that invokes the optional SPI speed parameter as a keyword argument. The following patch would need to be applied if this is merged.

-radio = RF24(CE_PIN, CSN_PIN, spispeed=4000000)
+radio = RF24(CE_PIN, CSN_PIN, spi_speed=4000000)

although any code that invokes it as a positional argument should still work fine

radio = RF24(CE_PIN, CSN_PIN, 4000000)

I'm betting that most users are doing the second usage (if at all changing the default SPI speed) because the C++ docs (including COMMON_ISSUES.md) suggest doing it this way.

Copy link
Member Author

Choose a reason for hiding this comment

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

IIRC, boost.python does not handle keyword arguments gracefully. It may be safe to merge this if spispeed=<int> never worked previously.

Copy link
Contributor

github-actions bot commented Sep 8, 2024

Memory usage change @ 096d5b9

Board flash % RAM for global variables %
arduino:avr:nano 0 - 0 0.0 - 0.0 0 - 0 0.0 - 0.0
arduino:samd:mkrzero 💚 -8 - 0 -0.0 - 0.0 0 - 0 0.0 - 0.0
Click for full report table
Board examples/GettingStarted
flash
% examples/GettingStarted
RAM for global variables
% examples/AcknowledgementPayloads
flash
% examples/AcknowledgementPayloads
RAM for global variables
% examples/ManualAcknowledgements
flash
% examples/ManualAcknowledgements
RAM for global variables
% examples/StreamingData
flash
% examples/StreamingData
RAM for global variables
% examples/MulticeiverDemo
flash
% examples/MulticeiverDemo
RAM for global variables
% examples/InterruptConfigure
flash
% examples/InterruptConfigure
RAM for global variables
% examples/scanner
flash
% examples/scanner
RAM for global variables
% examples/encodeRadioDetails
flash
% examples/encodeRadioDetails
RAM for global variables
%
arduino:avr:nano 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0
arduino:samd:mkrzero 0 0.0 0 0.0 -8 -0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0
Click for full report CSV
Board,examples/GettingStarted<br>flash,%,examples/GettingStarted<br>RAM for global variables,%,examples/AcknowledgementPayloads<br>flash,%,examples/AcknowledgementPayloads<br>RAM for global variables,%,examples/ManualAcknowledgements<br>flash,%,examples/ManualAcknowledgements<br>RAM for global variables,%,examples/StreamingData<br>flash,%,examples/StreamingData<br>RAM for global variables,%,examples/MulticeiverDemo<br>flash,%,examples/MulticeiverDemo<br>RAM for global variables,%,examples/InterruptConfigure<br>flash,%,examples/InterruptConfigure<br>RAM for global variables,%,examples/scanner<br>flash,%,examples/scanner<br>RAM for global variables,%,examples/encodeRadioDetails<br>flash,%,examples/encodeRadioDetails<br>RAM for global variables,%
arduino:avr:nano,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0
arduino:samd:mkrzero,0,0.0,0,0.0,-8,-0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0

@@ -0,0 +1,313 @@
version: "0.2"
language: en
words:
Copy link
Member Author

@2bndy5 2bndy5 Sep 8, 2024

Choose a reason for hiding this comment

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

Tip

Any intentionally misspelled words are specified in this yaml list.
The cSpell VSCode extension provides a way to auto-add any intentionally misspelled words to this list using only the mouse (no need to manually type it in here).

Notice this list is sorted alphabetically.

@TMRh20
Copy link
Member

TMRh20 commented Sep 8, 2024

Damn, those are a pile of spelling mistakes!

@2bndy5
Copy link
Member Author

2bndy5 commented Sep 8, 2024

And there's over 280 words intentionally misspelled including proper nouns (like products, brands, people's name/username) and some acronyms. Most of the exceptions are from not using snake casing or camel casing, so cspell can't identify the word "pigpio" in the phrase "libpigpio" (both would have to be ignored misspellings).

@2bndy5 2bndy5 marked this pull request as ready for review September 9, 2024 08:16
@2bndy5 2bndy5 merged commit 233d865 into master Sep 13, 2024
152 checks passed
@2bndy5 2bndy5 deleted the fix-spelling branch September 13, 2024 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants