Skip to content

DigitalOutputs writeAll function not correctly writing to each Output #8

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

Closed
kevmk04 opened this issue Mar 4, 2024 · 4 comments · Fixed by #10
Closed

DigitalOutputs writeAll function not correctly writing to each Output #8

kevmk04 opened this issue Mar 4, 2024 · 4 comments · Fixed by #10

Comments

@kevmk04
Copy link
Contributor

kevmk04 commented Mar 4, 2024

When using the writeAll function for Digital Outputs, the function will only turn on/off all Outputs regardless of input integer. For example, supplying any value greater than zero will turn ON all Outputs. Providing a zero will turn OFF all Outputs.

Function source code is here:

void DigitalOutputsClass::writeAll(uint8_t val_mask) {

Within the function, it appears that the logical (&&) operator is being used where a bitwise (&) should be. I was able to test this with a Portenta Machine Control and confirmed that switching the operator fixes the issue and allows the integer to be correctly parsed into the individual outputs.

Line w/ Operator:

@kevmk04
Copy link
Contributor Author

kevmk04 commented Mar 4, 2024

I'm happy to create a PR for this fix if desired.

@facchinm
Copy link
Contributor

facchinm commented Mar 5, 2024

Ah good catch, can you submit a PR that I'll merge immediately? Just to keep track of the authorship 🙂

@leonardocavagnis
Copy link
Contributor

Dear @kevmk04,
We're waiting for the PR 😉

@kevmk04
Copy link
Contributor Author

kevmk04 commented Mar 18, 2024

Sorry for the delay, missed the notifications. PR is up for your review!

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 a pull request may close this issue.

3 participants