Mirageofmage commented 1 year ago

This PR adds functionality for the Gain set function of the ADC, as well as fixes an issue that prevents the library from working on the ESP32-S3 modules

nerdyscout commented 1 year ago

I merged those changes to the develop branch. feel free to test.

nerdyscout commented 1 year ago

could you please confirm to removeable of no/interrupts()? This seems wrong to me, I had some serious issues here!
I would prefer to have seperate commits for each things clearly documented. It is also hard for me to stick to this....
I am currently working on some changes which will break you latest contribution, therefore currently will not merge anything further.

LeKrakenTheCode commented 7 months ago

Hey, I am trying to get this library to work with an ESP32-S3-mini, which should be the same as just the S3. It keeps freezing when the analogRead(MCP_CH0) is called. I would be happy to give more info if needed. Is this the same issue as mentioned above and if so can I help troubleshoot?

nerdyscout commented 7 months ago

Hi @LeKrakenTheCode! I would be really happy to close this issue, so every info is highly welcomed. do you run main or develop?

LeKrakenTheCode commented 7 months ago

I am running main currently. I have made the changes to the struct as well as added the arguments to the begin statement on line 96 of the cpp file. That has fixed my "freeze" (which now i realize was the endless loop mentioned above), however I am still not getting any values other than zero with the analogread and conversion. Thanks in advance, your library is super well written. Should I move to the develop branch?

Mirageofmage commented 7 months ago

I am running main currently. I have made the changes to the struct as well as added the arguments to the begin statement on line 96 of the cpp file. That has fixed my "freeze" (which now i realize was the endless loop mentioned above), however I am still not getting any values other than zero with the analogread and conversion. Thanks in advance, your library is super well written. Should I move to the develop branch?

Did you also update the union in the header file? Check the updates in this commit

The develop branch has all of the changes made in this PR and should work on the S3

LeKrakenTheCode commented 7 months ago

I am running main currently. I have made the changes to the struct as well as added the arguments to the begin statement on line 96 of the cpp file. That has fixed my "freeze" (which now i realize was the endless loop mentioned above), however I am still not getting any values other than zero with the analogread and conversion. Thanks in advance, your library is super well written. Should I move to the develop branch?

Did you also update the union in the header file? Check the updates in this commit

I have updated the union (i said struct by mistake). I added _status = read(&adcdata); to the analogRead function after the while loop as well. As I am using the MCP3462T-E/NC I also changed the reference to 2.4 from 3.3 if that matters. I found one mistake i have made, I meant to use the MCP3462R which has an internal reference but i used the T. So right now the ADC has nothing connected to the reference pins which is probably why i cannot get a voltage read.

nerdyscout commented 7 months ago

it has been a while sine working last on this project... so hard to remember all the pitfalls..

Develop contains breaking changes why I would always suggest to use it, otherwise you might run into issues once updating the library. It should be stable enough, at least I use it in my project for a while now... but I experienced recently something very similar problem with develop, but not in all cases...

Yes, if you are really using a version without R a external reference must be connected and configured!

LeKrakenTheCode commented 6 months ago

I got in the correct chip now with internal reference. I tried with the changes I made to the main branch code, but still am not getting any voltage back. I moved to the develop branch and I had to update the union again along with the spi->begin arguments. Back to the same issue where i simply don't get any value other than 0 from the analogRead.

LeKrakenTheCode commented 6 months ago

I got in the correct chip now with internal reference. I tried with the changes I made to the main branch code, but still am not getting any voltage back. I moved to the develop branch and I had to update the union again along with the spi->begin arguments. Back to the same issue where i simply don't get any value other than 0 from the analogRead.

Hope you had a good holiday!. I have done more testing and can confirm I have the pins correct as if one of them is not correct it doesn't finish the analog read. So I have been checking the config registers next and still can't find any problems. Can you think of any reasons I would only be getting an analogRead of 0? The reference is now enabled in the correct chip. (according to the registers)

Mirageofmage commented 6 months ago

Are you measuring in single ended or differential mode? Do you have a code sample and schematic?

On Jan 22, 2024 at 3:36 PM -0500, Thomas McMillan @.***>, wrote:

I got in the correct chip now with internal reference. I tried with the changes I made to the main branch code, but still am not getting any voltage back. I moved to the develop branch and I had to update the union again along with the spi->begin arguments. Back to the same issue where i simply don't get any value other than 0 from the analogRead. Hope you had a good holiday!. I have done more testing and can confirm I have the pins correct as if one of them is not correct it doesn't finish the analog read. So I have been checking the config registers next and still can't find any problems. Can you think of any reasons I would only be getting an analogRead of 0? The reference is now enabled in the correct chip. (according to the registers) — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: @.***>

LeKrakenTheCode commented 6 months ago

I have tried both reading channel 1 using MUX_CH1 and the built in temperature. Currently it is this:

Mirageofmage commented 6 months ago

I have tried both reading channel 1 using MUX_CH1 and the built in temperature. Currently it is this:

Did you call adc.begin() in setup?

LeKrakenTheCode commented 6 months ago

I have tried both reading channel 1 using MUX_CH1 and the built in temperature. Currently it is this:

Did you call adc.begin() in setup?

yes, forgot to include that:

nerdyscout commented 6 months ago

REFin- should be connected to GNDA
GNDD and GNDA should be somwhere connected
atleast on develop you should use adc.begin(0), not setting Vref! begin() will internally call setReference(vref) which enables internal reference as vref defaults to 0

void MCP3x6x::setReference(float vref) {
  if (vref == 0.0) {
    vref                      = 2.4;
    settings.config0.vref_sel = 1;
    _status                   = write(settings.config0);
  }
  _reference = vref;
}

anyway... none of these really explains getting zero values, I would expect noise but not zeros.

LeKrakenTheCode commented 6 months ago

@.***

I do have AGND and GND connected via a ferrite. Good catch with the AGND on REFIN-, Thank you. I was also expecting some noise as well. Especially at 16 bit.

From: nerdyscout @.> Sent: Tuesday, January 23, 2024 10:57 AM To: nerdyscout/Arduino_MCP3x6x_Library @.> Cc: Thomas McMillan @.>; Mention @.> Subject: Re: [nerdyscout/Arduino_MCP3x6x_Library] Adding functionality and ESP32-S3 support (PR #5)

REFin- should be connected to GNDA
GNDD and GNDA should be somwhere connected
atleast on develop you should use adc.begin(0), not setting Vref! begin() will internally call setReference(vref) which enables internal reference as vref defaults to 0

void MCP3x6x::setReference(float vref) {

if (vref == 0.0) {

vref                      = 2.4;

settings.config0.vref_sel = 1;

_status                   = write(settings.config0);

}

_reference = vref;

}

anyway... none of these really explains getting zero values, I would expect noise but not zeros.

— Reply to this email directly, view it on GitHubhttps://github.com/nerdyscout/Arduino_MCP3x6x_Library/pull/5#issuecomment-1906507665, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ALLYL4LJSP7HM4XWYONJFTDYP7TVPAVCNFSM6AAAAAASZI6FHWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMBWGUYDONRWGU. You are receiving this because you were mentioned.Message ID: @.**@.>>

Mirageofmage commented 6 months ago

@.*** I do have AGND and GND connected via a ferrite. Good catch with the AGND on REFIN-, Thank you. I was also expecting some noise as well. Especially at 16 bit.

Your REF+ is floating? On my design I have that connected to 3V3

nerdyscout commented 6 months ago

Are you measuring in single ended or differential mode?

and Mux mode or continuous mode?

LeKrakenTheCode commented 6 months ago

If I connect an external supply to refin+, it might override the internal reference. @.***

So I left that pin floating as I don’t need to use the reference anywhere else.

From: Jeremiah Ukwela @.> Sent: Tuesday, January 23, 2024 11:08 AM To: nerdyscout/Arduino_MCP3x6x_Library @.> Cc: Thomas McMillan @.>; Mention @.> Subject: Re: [nerdyscout/Arduino_MCP3x6x_Library] Adding functionality and ESP32-S3 support (PR #5)

@.*** I do have AGND and GND connected via a ferrite. Good catch with the AGND on REFIN-, Thank you. I was also expecting some noise as well. Especially at 16 bit.

Your REF+ is floating? On my design I have that connected to 3V3

— Reply to this email directly, view it on GitHubhttps://github.com/nerdyscout/Arduino_MCP3x6x_Library/pull/5#issuecomment-1906530818, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ALLYL4PYIEOPXWBOUKVICRDYP7U6JAVCNFSM6AAAAAASZI6FHWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMBWGUZTAOBRHA. You are receiving this because you were mentioned.Message ID: @.**@.>>

LeKrakenTheCode commented 6 months ago

The chip is sending “mux” to the serial output. Is there anything I need to do specifically to enable mux mode? It appeared like just calling analogRead ran as mux, but I could be wrong.

From: nerdyscout @.> Sent: Tuesday, January 23, 2024 11:09 AM To: nerdyscout/Arduino_MCP3x6x_Library @.> Cc: Thomas McMillan @.>; Mention @.> Subject: Re: [nerdyscout/Arduino_MCP3x6x_Library] Adding functionality and ESP32-S3 support (PR #5)

Are you measuring in single ended or differential mode?

and Mux mode or continuous mode?

— Reply to this email directly, view it on GitHubhttps://github.com/nerdyscout/Arduino_MCP3x6x_Library/pull/5#issuecomment-1906533616, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ALLYL4LO6VWWPOCH2OJP3E3YP7VDNAVCNFSM6AAAAAASZI6FHWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMBWGUZTGNRRGY. You are receiving this because you were mentioned.Message ID: @.**@.>>

nerdyscout commented 6 months ago

yes MuxMode should be fine by default, but I never used it why I am lacking experience.

int32_t MCP3x6x::analogRead(mux_t ch) {
  // MuxMode
  if (settings.scan.channel.raw == 0) {
#ifdef MCP3x6x_DEBUG
    Serial.println("mux");
#endif
    settings.mux = ch;
    _status      = write(settings.mux);
    _status      = conversion();
    while (!_status.dr) {
      _status = read(&adcdata);
    }
    return result.raw[(uint8_t)adcdata.channelid] = adcdata.value;
  }

freezing in here should only be possible if the while() is never returned. best would be if you could connect a debugger and step through to see where the read() fails.

could you please double check if you got this ":1" which @Mirageofmage fixed ? this is essential and would explain freezes.

  typedef union __attribute__((__packed__)) {
    struct {
      struct {
        bool por:1;     //!< status: power on reset
        bool crccfg:1;  //!< status: crc
        bool dr:1;      //!< status: data ready
      };
      uint8_t      : 1;  //!< !addr[0]
      uint8_t addr : 2;  //!< addresse
      uint8_t      : 2;  //!< EMTPY
    };
    uint8_t raw;
  } status_t;
  status_t _status;

LeKrakenTheCode commented 6 months ago

It is not freezing currently, just returning a zero. It only freezes if I have the wrong pins defined or do not have the chip connected. I have added the :1 changes: @.***

From: nerdyscout @.> Sent: Tuesday, January 23, 2024 11:51 AM To: nerdyscout/Arduino_MCP3x6x_Library @.> Cc: Thomas McMillan @.>; Mention @.> Subject: Re: [nerdyscout/Arduino_MCP3x6x_Library] Adding functionality and ESP32-S3 support (PR #5)

yes MuxMode should be fine by default, but I never used it why I am lacking experience.

int32_t MCP3x6x::analogRead(mux_t ch) {

// MuxMode

if (settings.scan.channel.raw == 0) {

ifdef MCP3x6x_DEBUG

Serial.println("mux");

endif

settings.mux = ch;

_status      = write(settings.mux);

_status      = conversion();

while (!_status.dr) {

  _status = read(&adcdata);

}

return result.raw[(uint8_t)adcdata.channelid] = adcdata.value;

}

freezing in here should only be possible if the while() is never returned. best would be if you could connect a debugger and step through to see where the read() fails.

could you please double check if you got this ":1" which @Mirageofmagehttps://github.com/Mirageofmage fixed ? this is essential and would explain freezes.

typedef union attribute((packed)) {

struct {

  struct {

    bool por:1;     //!< status: power on reset

    bool crccfg:1;  //!< status: crc

    bool dr:1;      //!< status: data ready

  };

  uint8_t      : 1;  //!< !addr[0]

  uint8_t addr : 2;  //!< addresse

  uint8_t      : 2;  //!< EMTPY

};

uint8_t raw;

} status_t;

status_t _status;

— Reply to this email directly, view it on GitHubhttps://github.com/nerdyscout/Arduino_MCP3x6x_Library/pull/5#issuecomment-1906607357, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ALLYL4MLS2YKWHBC4ZQTGXDYP72BJAVCNFSM6AAAAAASZI6FHWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMBWGYYDOMZVG4. You are receiving this because you were mentioned.Message ID: @.**@.>>

nerdyscout commented 6 months ago

\\ line 273
  if (settings.scan.channel.raw == 0) {
#ifdef MCP3x6x_DEBUG
    Serial.println("mux");
#endif
    settings.mux = ch;
    _status      = write(settings.mux);
    _status      = conversion();
    while (!_status.dr) {
      _status = read(&adcdata);
    }
    return result.raw[(uint8_t)adcdata.channelid] = adcdata.value;
  }

just another wild guess: try removing _status = on conversion() and write(settings.mux) maybe even but the conversion() into the loop

LeKrakenTheCode commented 6 months ago

@. Still getting zero on the output: @.

From: nerdyscout @.> Sent: Tuesday, January 23, 2024 12:44 PM To: nerdyscout/Arduino_MCP3x6x_Library @.> Cc: Thomas McMillan @.>; Mention @.> Subject: Re: [nerdyscout/Arduino_MCP3x6x_Library] Adding functionality and ESP32-S3 support (PR #5)

\ line 273

if (settings.scan.channel.raw == 0) {

ifdef MCP3x6x_DEBUG

Serial.println("mux");

endif

settings.mux = ch;

_status      = write(settings.mux);

_status      = conversion();

while (!_status.dr) {

  _status = read(&adcdata);

}

return result.raw[(uint8_t)adcdata.channelid] = adcdata.value;

}

just another wild guess: try replacing _status = conversion(); with conversion();

— Reply to this email directly, view it on GitHubhttps://github.com/nerdyscout/Arduino_MCP3x6x_Library/pull/5#issuecomment-1906702513, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ALLYL4NQXEVAIVRXTAE36R3YQAAILAVCNFSM6AAAAAASZI6FHWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMBWG4YDENJRGM. You are receiving this because you were mentioned.Message ID: @.**@.>>

Mirageofmage commented 6 months ago

What's the speed of your SPI bus? I remember having some issues if the speed was over 500k. I'm currently running w/ 200k

Mirageofmage commented 6 months ago

Also, @LeKrakenTheCode should probably open a new issue while this can be pulled into branch

nerdyscout / Arduino_MCP3x6x_Library

Adding functionality and ESP32-S3 support #5

ifdef MCP3x6x_DEBUG

endif

ifdef MCP3x6x_DEBUG

endif