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

setComparatorPolarity and setComparatorLatch are inverting the setting? #80

Closed
HineSite opened this issue Aug 20, 2024 · 9 comments · Fixed by #81
Closed

setComparatorPolarity and setComparatorLatch are inverting the setting? #80

HineSite opened this issue Aug 20, 2024 · 9 comments · Fixed by #81
Assignees
Labels
bug Something isn't working

Comments

@HineSite
Copy link

HineSite commented Aug 20, 2024

I could be wrong, but I don't think these are correct?

void ADS1X15::setComparatorPolarity(uint8_t pol)
{
  _compPol = pol ? 0 : 1;
}

void ADS1X15::setComparatorLatch(uint8_t latch)
{
  _compLatch = latch ? 0 : 1;
}

Based on how the registers are being set later:

  if (_compPol)   config |= ADS1X15_COMP_POL_ACTIV_HIGH;      //  bit 3      ALERT active value
  else            config |= ADS1X15_COMP_POL_ACTIV_LOW;
  if (_compLatch) config |= ADS1X15_COMP_LATCH;
  else            config |= ADS1X15_COMP_NON_LATCH;           //  bit 2      ALERT latching

Example:

  ADS.setComparatorPolarity(1);
  ADS.setComparatorLatch(1);

  ADS.readADC(0);

  Serial.println(
    "Polarity: " + String(ADS.getComparatorPolarity()) + 
    " | Latch: " + String(ADS.getComparatorLatch())
  );

Results: Polarity: 0 | Latch: 0

@RobTillaart RobTillaart self-assigned this Aug 20, 2024
@RobTillaart RobTillaart added question Further information is requested bug Something isn't working and removed question Further information is requested labels Aug 20, 2024
@RobTillaart
Copy link
Owner

Thanks for the issue,
As I am quite busy on some other tasks, it might take several days (weeks) before I can dive into it.
Definitely looks like a bug given your code snippet, so I labeled it as such to give it priority it needs.

@RobTillaart
Copy link
Owner

@HineSite
Can you run this example as I have to test setup near me.
Please post the output.

It checks all the "configuration flags".

//
//    FILE: ADS_test_config.ino
//  AUTHOR: Rob.Tillaart
// PURPOSE: test config flags
//     URL: https://github.com/RobTillaart/ADS1X15
//          triggered by issue 80


#include "ADS1X15.h"

ADS1115 ADS(0x48);

void setup()
{
  Serial.begin(115200);
  Serial.println(__FILE__);
  Serial.print("ADS1X15_LIB_VERSION: ");
  Serial.println(ADS1X15_LIB_VERSION);

  Wire.begin();

  ADS.begin();

  Serial.println("\nTEST GAIN");
  int gain = 16;
  for (int i = 0; i < 6; i++)
  {
    Serial.print(gain);
    ADS.setGain(gain);
    if (ADS.getGain() == gain) Serial.println("\tOK");
    else Serial.println("\tFAIL");
  }

  
  Serial.println("\nTEST DATARATE");
  for (int i = 0; i < 7; i++)
  {
    Serial.print(i);
    ADS.setDataRate(i);
    if (ADS.getDataRate() == i) Serial.println("\tOK");
    else Serial.println("\tFAIL");
  }

  Serial.println("\nTEST MODE");
  for (int i = 0; i < 2; i++)
  {
    Serial.print(i);
    ADS.setMode(i);
    if (ADS.getMode() == i) Serial.println("\tOK");
    else Serial.println("\tFAIL");
  }

  Serial.println("\nTEST COMP MODE");
  for (int i = 0; i < 2; i++)
  {
    Serial.print(i);
    ADS.setComparatorMode(i);
    if (ADS.getComparatorMode() == i) Serial.println("\tOK");
    else Serial.println("\tFAIL");
  }


  Serial.println("\nTEST COMP POLARITY");
  for (int i = 0; i < 2; i++)
  {
    Serial.print(i);
    ADS.setComparatorPolarity(i);
    if (ADS.getComparatorPolarity() == i) Serial.println("\tOK");
    else Serial.println("\tFAIL");
  }

  Serial.println("\nTEST COMP LATCH");
  for (int i = 0; i < 2; i++)
  {
    Serial.print(i);
    ADS.setComparatorLatch(i);
    if (ADS.getComparatorLatch() == i) Serial.println("\tOK");
    else Serial.println("\tFAIL");
  }


  Serial.println("\nTEST COMP QUECONVERT");
  for (int i = 0; i < 2; i++)
  {
    Serial.print(i);
    ADS.setComparatorQueConvert(i);
    if (ADS.getComparatorQueConvert() == i) Serial.println("\tOK");
    else Serial.println("\tFAIL");
  }


  Serial.println("\ndone...");
}

void loop()
{
}

//  -- END OF FILE --

@RobTillaart
Copy link
Owner

Note to myself: check if unit tests can be added

@RobTillaart
Copy link
Owner

@HineSite
Created a develop branch with a preliminary fix,
If you have time, please verify if it works as expected.

If the readme.md needs additional information on latch/polarity topic feel free to provide such.
(now back to my other tasks)

@RobTillaart
Copy link
Owner

@HineSite
Build of develop branch is stable, created a PR for release 0.5.0

@RobTillaart RobTillaart linked a pull request Aug 20, 2024 that will close this issue
@HineSite
Copy link
Author

HineSite commented Aug 20, 2024

This is the results when ran against 0.4.5

TEST GAIN
16	OK
16	OK
16	OK
16	OK
16	OK
16	OK

TEST DATARATE
0	OK
1	OK
2	OK
3	OK
4	OK
5	OK
6	OK

TEST MODE
0	OK
1	OK

TEST COMP MODE
0	OK
1	OK

TEST COMP POLARITY
0	FAIL
1	FAIL

TEST COMP LATCH
0	FAIL
1	FAIL

TEST COMP QUECONVERT
0	OK
1	OK

done...

@RobTillaart
Copy link
Owner

Thanks for running the testcode
That confirms the bug and shows there are no other inverted flags (as far as test goes).

@HineSite
Copy link
Author

Here are the results against the development branch.

ADS1X15_LIB_VERSION: 0.5.0

TEST GAIN
16	OK
16	OK
16	OK
16	OK
16	OK
16	OK

TEST DATARATE
0	OK
1	OK
2	OK
3	OK
4	OK
5	OK
6	OK

TEST MODE
0	OK
1	OK

TEST COMP MODE
0	OK
1	OK

TEST COMP POLARITY
0	OK
1	OK

TEST COMP LATCH
0	OK
1	OK

TEST COMP QUECONVERT
0	OK
1	OK

RobTillaart added a commit that referenced this issue Aug 21, 2024
- Fix #80, setComparatorPolarity() and setComparatorLatch() inverting.
- add test example to test parameters.
- add unit tests to test parameters.
@RobTillaart
Copy link
Owner

@HineSite
Merged PR into master,
Again, thanks for reporting this issue, appreciated!

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