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

Gather canned message magic numbers into header defines. #4623

Merged
merged 2 commits into from
Sep 6, 2024

Conversation

robertfisk
Copy link
Contributor

The canned message module accepts a bunch of magic numbers from input modules to perform system actions and navigation. This PR bundles them together as header defines for easier reference in CannedMessageModule.h.

Note that we are not touching any magic numbers used internally by the keyboards / input modules, or any standard ascii control characters.

@fifieldt
Copy link
Contributor

fifieldt commented Sep 4, 2024

This is a good idea, many thanks for submitting it.

I'm not so familiar with this module, but two questions:

  1. src/modules/CannedMessageModule.h is wrapped in #ifdef HAS_SCREEN , so just including the header doesn't necessarily result in these variables being available. If there's a chance any of the places these variables need to be used could be triggered on a device without a screen, this should be addressed.
  2. Canned Message Module seems like a convenient place to put this, but are there other options worth considering?

@caveman99
Copy link
Member

caveman99 commented Sep 4, 2024

These could go int inputbroker as well, since we may need them without canned messages in the future. I'd like to move any input device into inputbroker. Canned Message is the only client at the moment, but doesn't need to stay the only client.

@robertfisk
Copy link
Contributor Author

I've moved the defines to InputBroker.h now as suggested. This way doesn't require an additional #include in the various keyboards.

@HarukiToreda
Copy link
Contributor

I've moved the defines to InputBroker.h now as suggested. This way doesn't require an additional #include in the various keyboards.

would this perhaps fix a known issue when the environmental screen is enabled the left and righ buttons on the keyboard become unresponsive and can't switch to the next or previous frames? or is this an unrelated issue?

@caveman99 caveman99 force-pushed the tidyup_keyboard_defines branch from 2d48d80 to 962d9ff Compare September 6, 2024 09:54
@caveman99 caveman99 merged commit 35a565c into meshtastic:master Sep 6, 2024
104 checks passed
@robertfisk
Copy link
Contributor Author

would this perhaps fix a known issue when the environmental screen is enabled the left and righ buttons on the keyboard become unresponsive and can't switch to the next or previous frames? or is this an unrelated issue?

I'm not familiar with the environment screen or that bug sorry. I suspect it is unrelated.

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.

4 participants