Skip to content

ModulinoKnob: report error and not last valid _pressed if read() fails #20

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

Open
facchinm opened this issue Feb 6, 2025 · 1 comment
Assignees

Comments

@facchinm
Copy link
Contributor

facchinm commented Feb 6, 2025

Modulino/src/Modulino.h

Lines 273 to 275 in cf5e2b2

bool isPressed() {
get();
return _pressed;

@AntonioBerna
Copy link

🧠 Overview

The ModulinoKnob::get() method currently has a bug where it properly returns 0 when the I2C read operation fails, but it fails to update the internal _pressed state variable. This means that subsequent calls to isPressed() will return whatever the last successfully read button state was, rather than indicating that the state is unknown/invalid.

✅ Expected behavior

When I2C communication fails, both the position value and the button state should be invalidated.

❌ Current behavior

If I2C communication fails, the position is reset to 0, but the button state remains at its last known valid value.

🎮 Reproduction

  1. Successfully read a knob with button pressed (_pressed = true)
  2. Disconnect the I2C bus or cause another communication failure
  3. Call get() - returns 0 as expected
  4. Call isPressed() - incorrectly returns true when it should acknowledge the failure

✅ Proposed solution

Update the get() method to reset _pressed to false when read fails:

int16_t get() {
  uint8_t buf[3];
  auto res = read(buf, 3);
  if (res == false) {
    _pressed = false;  // Reset pressed state on read failure
    return 0;
  }
  _pressed = (buf[2] != 0);
  int16_t ret = buf[0] | (buf[1] << 8);
  return ret;
}

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

No branches or pull requests

3 participants