Qmc5883 #14714
Qmc5883 #14714
Conversation
|
This PR is technically for the QMC5883. I don't have an HMC5883L to test with, I misnamed the branch when I created it. |
|
Hi, Thanks for sharing. This PR will fail for all languages versions except for English due to missing keys. Please, add the missing keys to the rest of language files. Don't need to translate it. Just copy the same you have in the English file. Thanks |
| // xsns_96_QMC5883.ino | ||
| #define D_JSON_MX "Compass X-Axis" | ||
| #define D_JSON_MY "Compass Y-Axis" | ||
| #define D_JSON_MZ "Compass Z-Axis" | ||
| #define D_JSON_HEADING "Compass Heading" |
There was a problem hiding this comment.
These are wrong names for JSON keys
A JSON key must be a valid variable name in Javascript with only alphanumeric characters + underscore and starting with an alpha character.
Spaces, dash or other separators are illegal.
Please fixe
There was a problem hiding this comment.
Technically, that is not a requirement for JSON keys (which can be any valid string), only if you "happen" to want to use it with backend tools enforcing restrictions on names. Tasmota even introduced an option to replace hyphen with underscore to appease such tools. Still, I'd regard it as "best practice" to avoid names causing hair loss with people wanting to use it with JavaScript or similar....
| bool ready; | ||
| uint8_t i2c_address; | ||
|
|
||
| } QMC5883; |
There was a problem hiding this comment.
Do not allocate statically the memory for your driver
This is taking useless amount of RAM when the driver is part of a build but not used
Use dynamic allocation as in the HMC5883 driver
| if (FUNC_INIT == function) { | ||
| QMC5883_Init(); | ||
| } | ||
| else if (QMC5883.ready) { |
There was a problem hiding this comment.
Similar, look at the HMC5883 driver. Use the dynamic pointer for that purpose instead of wasting a byte
|
This PR has been automatically marked as stale because it hasn't any activity in last few weeks. It will be closed if no further activity occurs. Thank you for your contributions. |
github-actions
bot
added
the
stale
|
This PR was automatically closed because of being stale. |
Description:
Related issue (if applicable): fixes #
Checklist:
NOTE: The code change must pass CI tests. Your PR cannot be merged unless tests pass