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: warning pedantic binary const #1595

Merged

Conversation

perigoso
Copy link
Contributor

Detailed description

Fix (some) warnings for -Wpedantic

See #1590 for context

Your checklist for this pull request

  • I've read the Code of Conduct
  • I've read the guidelines for contributing to this repository
  • It builds for hardware native (make PROBE_HOST=native)
  • It builds as BMDA (make PROBE_HOST=hosted)
  • I've tested it to the best of my ability
  • My commit messages provide a useful short description of what the commits do

Closing issues

@perigoso perigoso changed the title Fix/warning pedantic binary const Fix: warning pedantic binary const Aug 10, 2023
@perigoso perigoso mentioned this pull request Aug 10, 2023
43 tasks
binary constants are a GCC extension
binary constants are a GCC extension
@perigoso perigoso force-pushed the fix/warning-pedantic-binary-const branch from 15e3fc2 to 37e7ca9 Compare August 21, 2023 21:56
Copy link
Member

@dragonmux dragonmux left a comment

Choose a reason for hiding this comment

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

We'd been contemplating this PR a while now because we weren't sure if switching away from the binary constants was a good idea or not, but the more we've thought about it, the more we've realised that your solution in putting the binary after the morse code conversion for each letter makes perfect sense here.

It perhaps bares writing an explainer at some point on how the binary representation maps to the dots and dashes representation, and why this mapping was chosen, but for now - LGTM, merging.

@dragonmux dragonmux added this to the v1.10 milestone Aug 29, 2023
@dragonmux dragonmux added Enhancement General project improvement BMP Firmware Black Magic Probe Firmware (not PC hosted software) labels Aug 29, 2023
@dragonmux dragonmux merged commit 78b327d into blackmagic-debug:main Aug 29, 2023
6 checks passed
@perigoso perigoso deleted the fix/warning-pedantic-binary-const branch August 30, 2023 08:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BMP Firmware Black Magic Probe Firmware (not PC hosted software) Enhancement General project improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants