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

TCA9548::disableChannel(uint8_t channel) is not working as expected #20

Closed
pghj opened this issue Apr 14, 2024 · 11 comments · Fixed by #21
Closed

TCA9548::disableChannel(uint8_t channel) is not working as expected #20

pghj opened this issue Apr 14, 2024 · 11 comments · Fixed by #21
Assignees
Labels
bug Something isn't working

Comments

@pghj
Copy link

pghj commented Apr 14, 2024

I think disableChannel was copy-pasted from enableChannel, but the condition !isEnabled(channel) was not inverted by mistake.

@RobTillaart RobTillaart self-assigned this Apr 14, 2024
@RobTillaart RobTillaart added the bug Something isn't working label Apr 14, 2024
@RobTillaart
Copy link
Owner

Thank you for catching,
definitely a (copy paste) BUG.
Will fix asap, probably tomorrow.

@pghj
Copy link
Author

pghj commented Apr 14, 2024

Thanks for the quick response.
It seems to me that the check can just be removed from both methods, because setChannelMask already tests the entire mask for changes.

@pghj pghj closed this as completed Apr 14, 2024
@pghj
Copy link
Author

pghj commented Apr 14, 2024

Oops wrong button.

@pghj pghj reopened this Apr 14, 2024
@RobTillaart
Copy link
Owner

It seems to me that the check can just be removed from both methods, because setChannelMask already tests the entire mask for changes.

Good idea, would simplify code a bit.
Will create a develop branch asap to prepare for a 0.3.0 version.

@RobTillaart
Copy link
Owner

Propose code
would also return false if the writing to the multiplexer failed.

bool TCA9548::enableChannel(uint8_t channel)
{
  if (channel >= _channels) return false;
  return setChannelMask(_mask | (0x01 << channel));
}

bool TCA9548::disableChannel(uint8_t channel)
{
  if (channel >= _channels) return false;
  return setChannelMask(_mask & ~(0x01 << channel));
}

RobTillaart added a commit that referenced this issue Apr 15, 2024
@RobTillaart
Copy link
Owner

@pghj
Develop branch created, build-CI is running.
Some more edits were made.

RobTillaart added a commit that referenced this issue Apr 15, 2024
@pghj
Copy link
Author

pghj commented Apr 15, 2024

Propose code

Looks good. Can you also make it return the result of setChannelMask when calling selectChannel?

@RobTillaart
Copy link
Owner

Will check after lunch

RobTillaart added a commit that referenced this issue Apr 15, 2024
@RobTillaart
Copy link
Owner

Propose code

Looks good. Can you also make it return the result of setChannelMask when calling selectChannel?

done

@RobTillaart
Copy link
Owner

@pghj
Do you have time to test the develop branch, otherwise I will merge it (as I see no issues for now).

@RobTillaart RobTillaart linked a pull request Apr 18, 2024 that will close this issue
RobTillaart added a commit that referenced this issue Apr 18, 2024
- Fix #20, **disableChannel()** bug + optimize.
- Fix **isConnected(address, channel)** bug.
- update keywords.txt
- update readme.md
- add **uint8_t find(address)** function, returns mask.
- update / add examples.
@RobTillaart
Copy link
Owner

@pghj
I merged develop and released 0.3.0 .

Again thanks for reporting the issue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants