nerdyscout / Arduino_MCP3x6x_Library

Library to support Microchip MPC3x6x 16/24bit analog to digital converters.
https://nerdyscout.github.io/Arduino_MCP3x6x_Library
MIT License
17 stars 9 forks source link

Get stuck in mcp.analogRead loop #4

Closed FelixWT closed 1 year ago

FelixWT commented 1 year ago

Hi, Thank you for making this ADC library. It's one of the few library that support MCP3562.

But when I'm implement this code to my ESP32 S3 it seems to stuck in analogread infinite loop. The differences are I make the MCP3562 instance aka mcp a global instance by adding extern MCP3562 mcp; in MCP3x6x.h and MCP3562 mcp=MCP3562(); in MCP3x6x.cpp . Also dude to the layout error I swap the pin definition of MISO & MOSI in the class Constructor .

MCP3x6x::MCP3x6x(const uint16_t MCP3x6x_DEVICE_TYPE, const uint8_t pinCS, SPIClass *theSPI,
                 const uint8_t pinMOSI, const uint8_t pinMISO, const uint8_t pinCLK) {
  switch (MCP3x6x_DEVICE_TYPE) {
    case MCP3461_DEVICE_TYPE:
      _resolution_max = 16;
      _channels_max   = 2;
      break;
    case MCP3462_DEVICE_TYPE:
      _resolution_max = 16;
      _channels_max   = 4;
      break;
    case MCP3464_DEVICE_TYPE:
      _resolution_max = 16;
      _channels_max   = 8;
      break;
    case MCP3561_DEVICE_TYPE:
      _resolution_max = 24;
      _channels_max   = 2;
      break;
    case MCP3562_DEVICE_TYPE:
      _resolution_max = 24;
      _channels_max   = 4;
      break;
    case MCP3564_DEVICE_TYPE:
      _resolution_max = 24;
      _channels_max   = 8;
      break;
    default:
#warning "undefined MCP3x6x_DEVICE_TYPE"
      break;
  }

  //  settings.id = MCP3x6x_DEVICE_TYPE;
/* ========================================= */

  _spi        = theSPI;
  _pinMISO    = pinMOSI;//SWAP DUE TO LAYOUT ERROR
  _pinMOSI    = pinMISO;//SWAP DUE TO LAYOUT ERROR
  _pinCLK     = pinCLK;
  _pinCS      = pinCS;

  Serial.print("_pinMISO: ");
  Serial.println(_pinMISO);
  Serial.print("_pinMOSI: ");
  Serial.println(_pinMOSI);
  Serial.print("_pinCLK: ");
  Serial.println(_pinCLK);
  Serial.print("_pinCS: ");
  Serial.println(_pinCS);

/* ========================================= */

  _resolution = _resolution_max;
  _channel_mask |= 0xff << _channels_max;  // todo use this one
};

And this is my main task

#include <SPI.h>
#include "MCP3x6x.h"

void setup()
{  
  Serial.begin(115200);  
  while (!mcp.begin()) {
    Serial.println("failed to initialize MCP");
    delay(200);
  }
}
void loop()
{
  Serial.println("1");//stuck detect
  int32_t adcdata = mcp.analogRead(MCP_CH0);
  Serial.println("2");//stuck detect
  double voltage = adcdata * mcp.getReference() / mcp.getMaxValue();  
  Serial.print("voltage: ");
  Serial.println(voltage, 10);
  delay(1000);
}

And the Serial out put: _pinMISO: 11 _pinMOSI: 13 _pinCLK: 12 _pinCS: 10 1 mux

Any advice is greatly appreciated.

Mirageofmage commented 1 year ago

Hey Felix,

I've run into this exact issue (also with the ESP32S3). I'm currently working on a PR to make this library fully compatible ( GH-5), but for the time being here's the fix in order to get analogRead to work.

In MCP3x6x.h

typedef union {
     struct {
       struct __attribute__((__packed__)) {
         bool por;     //!< status: power on reset
         bool crccfg;  //!< status: crc
         bool dr;      //!< 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;

Must get updated to

typedef union {
     struct {
       struct __attribute__((__packed__)) {
         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;

There's an issue on the ESP32S3 specifically where the bit assignment in the union doesn't work properly unless explicitly stated.

You can also optionally add _status = read(&adcdata); after the

while (!_status.dr) {
       _status = read(&adcdata);
}

in analogRead; however, I'm not sure if this is necessary or not.

Best,

nerdyscout commented 1 year ago

could you please specify what you mean by: Also dude to the layout error in swap the pin definition of MISO & MOSI in the class Constructor . where did I swap something?

FelixWT commented 1 year ago

nerdyscout

could you please specify what you mean by: Also dude to the layout error in swap the pin definition of MISO & MOSI in the class Constructor . where did I swap something?

This is the problem on my end. It is dude to the wrong PCB layout of my project. thank you for your reply

FelixWT commented 1 year ago

Mirageofmage Thank you for your quick reply , I've tried what you suggest and it works now! But the readings keeps on increasing even when I do nothing to the device.

By the way, I should have mentioned this in my post that I'm using MCP3562R: dual channel variant.

This is my circuit: Curcit

I'm trying to read ADC readings from Device 1. So I'm assuming I should change MCP_CH0 definition in MCP3x6x.h from0x08 to 0x01

FelixWT commented 1 year ago

I manage to make it work now.

What I did was twick the configuration a bit setDataFormat(data_format::SGN_DATA); And change the reference voltage to 3.3. Also as I mentioned in my last post I change MCP_CH0definition inMCP3x6x.h from0x08 to0x01. And now I'm getting the readings almost 6.6 which is what it should be while off load.

Apart from the topic, shouldn't the code here

https://github.com/nerdyscout/Arduino_MCP3x6x_Library/blob/4d00da90100cf651cd27d3e3fcba1da4cd59bc75/lib/MCP3x6x/MCP3x6x.cpp#L96 be like this? _spi->begin(_pinCLK,_pinMISO,_pinMOSI,_pinCS); otherwise the pin configurations will be overwrite by default SPI configurations.

nerdyscout commented 1 year ago

There is no begin() in SPIClass which takes any parameters, therefore no :)

Please check the examples, there I use a setup with a custom SPI, this should also help with your pin swap without modifiying my lib.

SPIClass mySPI(&sercom1, 12, 13, 11, SPI_PAD_0_SCK_1, SERCOM_RX_PAD_3);
MCP3561 mcp(8, 7, 10, &mySPI, 11, 12, 13);
...
  _spi        = theSPI;
...
 _spi->begin(); 

As you can see the custom SPI mySPI is passed through the constructor into the privat member _spi which therefore got the correct pin definitions.

As far as I understand your schematic your try to do some differential reading as well as a single ended reading. There should be nothing wrong with this, but my personal approach would be to handle both the same way... anyway, you should not need to change the definition of MCP_CH0 but instead mcp.enableScanChannel(MCP_DIFFA); for enabling reading differntial signals and mcp.enableScanChannel(MCP_CH2) for reading pressure - altleast in ScanMode. In MuxMode just do int32_t adcdata = mcp.analogRead(MCP_DIFFA); or MCP_CH2 whenever you like.

FelixWT commented 1 year ago

There is no begin() in SPIClass which takes any parameters, therefore no :)

Well , it seem that Arduino core didn't provide it but in ESP32 core it does. Check out the following link for SPI library in esp32 core (http://github.com/espressif/arduino-esp32/blob/9275dbff942bb2ccce62632607500ad27dd0513a/libraries/SPI/src/SPI.h#L63) And yes I did try out your code as well. But again it seems to be Arduino and ESP32 core difference I can't get your configurations to work.

int32_t adcdata = mcp.analogRead(MCP_DIFFA);

Ah, you did provide the settings I want. This is great it feels weird to modifiy your library. Thanks for this amazing library!