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

Remove unused variables and make actionFlag a uint8_t #107

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

fwolfst
Copy link

@fwolfst fwolfst commented Apr 19, 2023

No description provided.

@fwolfst
Copy link
Author

fwolfst commented Apr 19, 2023

I hope the changes can still easily be merged to 9_9 (there is no ino file released yet). It looks as the compiler optimizes them away (minimal difference in .bin size), but relevant improvement for (human) readers.

@patience4711
Copy link
Owner

patience4711 commented May 12, 2023

@fwolfst Making actionFlag an uint8_t wasn't such a good idea after all. This could be set to a value of 301 which means overflow resulting in 45 which in turn caused the sendZB to send a rubbish message. Which crashed the Zigbee module.
Took me hours to figure that out because i didn't realize the consequences of using that uint8_t. So I changed 301 to 250 to solve it.

@fwolfst
Copy link
Author

fwolfst commented May 12, 2023

@fwolfst Making actionFlag an uint8_t wasn't such a good idea after all. This could be set to a value of 301 which means overflow resulting in 45 which in turn caused the sendZB to send a rubbish message. Which crashed the Zigbee module. Took me hours to figure that out because i didn't realize the consequences of using that uint8_t. So I changed 301 to 250 to solve it.

Oh gosh. Yeah, I should have checked that before. I had not much time, my code does not yet pair (but I get closer, command-by-command ....)

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