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

ESP32-S3-Mini and MCP3462R not working in Mux Mode #13

Open LeKrakenTheCode opened 6 months ago

LeKrakenTheCode commented 6 months ago

Keep receiving a value of zero from analogRead. Moving this into its own issue from: https://github.com/nerdyscout/Arduino_MCP3x6x_Library/pull/5#issuecomment-1870591920

LeKrakenTheCode 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\

I was running at 10MHz. I lowered to 200KHz and still no change.

Mirageofmage commented 6 months ago

If you have access to a scope, can you check the activity on MISO?

LeKrakenTheCode commented 6 months ago

Sorry for the delay. Was feeling rather unwell this weekend. Working on it now.

LeKrakenTheCode commented 5 months ago

If you have access to a scope, can you check the activity on MISO?

image image

I have this tool that can view SPI. Does this help? Please ignore the CS line, it is not connected as i didn't break out a header for that pin, however the analyzer is set to ignore the CS line for now.

nerdyscout commented 5 months ago

MISO and MOSI seem to show reasonable signals. Seems like the second picture is showing a 1Byte command followed by another 2Byte command... I did not try to figure out which are these. But it seems save to say MCP3x6x::read(Adcdata *data) is never executed as this would result in a 3-5Byte command. At least this is not shown in any of these pictures.

I still suspect the issue in this little snippet:

    settings.mux = ch;
    _status      = write(settings.mux);
    _status      = conversion();
    while (!_status.dr) {
      _status = read(&adcdata);
    }

please put an Serial.print(_status.dr) inside the loop and right in front of it, it looks like this is currently not executed. This bit should be set when conversion started and reset when valid data available. As this bit is part of the status byte endianness is important, but also timing of spi speed vs conversion speed might cause it.

LeKrakenTheCode commented 5 months ago

image Here's the change and here is the output: image only one appears to be running.

Mirageofmage commented 5 months ago

image Here's the change and here is the output: image only one appears to be running.

I've had issues with the while loop, in my implementation I just directly read instead of waiting for the data ready bit. I have to be careful with my sample rate or it returns the same value for each of the channels.

LeKrakenTheCode commented 5 months ago

image

No change to the output.

Mirageofmage commented 5 months ago
image
LeKrakenTheCode commented 5 months ago

still no change to the output:

image

Trying to read MCP_TEMP.

Mirageofmage commented 5 months ago

@nerdyscout

image

Question on why the int32 is truncated to 25 bits in adcdata? The ADC can return either left aligned or right aligned 32 bit values. The assignment in the result struct retains all 32 bits

nerdyscout commented 5 months ago

Question on why the int32 is truncated to 25 bits in adcdata? The ADC can return either left aligned or right aligned 32 bit values. The assignment in the result struct retains all 32 bits

yes, the dataformat is choosen within the begin() funtion

Mirageofmage commented 5 months ago

Question on why the int32 is truncated to 25 bits in adcdata? The ADC can return either left aligned or right aligned 32 bit values. The assignment in the result struct retains all 32 bits

yes, the dataformat is choosen within the begin() funtion

I see that, but my point is that if somebody chooses a left aligned output, then the 7 leftmost bits get masked out? Or am I understanding it wrong?

nerdyscout commented 5 months ago

I assume you are right. My intention was to make this library for getting easy started as the component is really complex. This is why the begin functions sets a few default parameters... they might be not good for every case, but if someone chooses to set up their own config than I would suspect it is on him self to handle this. This is also why I refactored settings to be a class by its own so in future it could be a parameter of begin().

LeKrakenTheCode commented 5 months ago

Any other idea's I can try?

Mirageofmage commented 5 months ago

Any other idea's I can try?

Maybe try returning adcdata.value directly at the end of analogRead?

So return adcdata.value; instead of return result.raw[(...

Otherwise I don't really have a clue/it has to be in hardware? I've been testing the same code revisions on my hardware and it's been working (though I'm not using the R revision).

I remember that you put the pins in the constructor of whichever MCP3x6x class you used, I instead opted to redefine the consts in the default constructor.

So my object declaration is just MCP3561 mcp;

with #defines for MOSI, MISO, SS, SCK, and IRQ (if required). The default SPI interface through Arduino is selected.

Mirageofmage commented 5 months ago

I actually think you HAVE to use #define to select pins instead of using the constructor, it doesn't look like parameters are properly passed through the constructor to the SPI object

The parameters required are

#define MOSI 
#define MISO
#define SS
#define SCK
nerdyscout commented 5 months ago

I actually think you HAVE to use #define to select pins instead of using the constructor, it doesn't look like parameters are properly passed through the constructor to the SPI object

The parameters required are

#define MOSI 
#define MISO
#define SS
#define SCK

as his logic analyzer pictures have proven some SPI communication I don't see why this should be the case.

Mirageofmage commented 5 months ago

If he's hooked up to the default pins then this isn't necessary, I'm just assuming not since he passed in parameters to the constructor. On Feb 12, 2024 at 1:07 PM -0500, nerdyscout @.***>, wrote:

I actually think you HAVE to use #define to select pins instead of using the constructor, it doesn't look like parameters are properly passed through the constructor to the SPI object The parameters required are

define MOSI

define MISO

define SS

define SCK

as his logic analyzer pictures have proven some SPI communication I don't see why this should be the case. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.Message ID: @.***>

nerdyscout commented 5 months ago

@LeKrakenTheCode please publish your code. either setup a new repository as minimal example which reproduces your issue or the original code.

LeKrakenTheCode commented 5 months ago

It has been published here:

https://github.com/McMillanFlowProducts/McMillanDevBoard/

From: nerdyscout @.> Sent: Monday, February 12, 2024 12:16 PM To: nerdyscout/Arduino_MCP3x6x_Library @.> Cc: Thomas McMillan @.>; Mention @.> Subject: Re: [nerdyscout/Arduino_MCP3x6x_Library] ESP32-S3-Mini and MCP3462R not working in Mux Mode (Issue #13)

@LeKrakenTheCodehttps://github.com/LeKrakenTheCode please publish your code. either setup a new repository as minimal example which reproduces your issue or the original code.

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

LeKrakenTheCode commented 5 months ago

I had to make a change to the line 69 in the cpp: _spi->begin(_pinCLK, _pinMISO, _pinMOSI, _pinCS); To get it to work with the ESP32 in the first place.

From: Jerry Ukwela @.> Sent: Monday, February 12, 2024 12:10 PM To: nerdyscout/Arduino_MCP3x6x_Library @.> Cc: Thomas McMillan @.>; Author @.> Subject: Re: [nerdyscout/Arduino_MCP3x6x_Library] ESP32-S3-Mini and MCP3462R not working in Mux Mode (Issue #13)

If he's hooked up to the default pins then this isn't necessary, I'm just assuming not since he passed in parameters to the constructor. On Feb 12, 2024 at 1:07 PM -0500, nerdyscout @.<mailto:@.>>, wrote:

I actually think you HAVE to use #define to select pins instead of using the constructor, it doesn't look like parameters are properly passed through the constructor to the SPI object The parameters required are

define MOSI

define MISO

define SS

define SCK

as his logic analyzer pictures have proven some SPI communication I don't see why this should be the case. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.Message ID: @.<mailto:@.>>

— Reply to this email directly, view it on GitHubhttps://github.com/nerdyscout/Arduino_MCP3x6x_Library/issues/13#issuecomment-1939276254, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ALLYL4JSCJJUUEC5K3NZHO3YTJLIRAVCNFSM6AAAAABCI7TINOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMZZGI3TMMRVGQ. You are receiving this because you authored the thread.Message ID: @.**@.>>

nerdyscout commented 5 months ago

seems like it could be fixed by something like this, will work on a better fix later.

MCP3x6x::status_t MCP3x6x::_transfer(uint8_t *data, uint8_t addr, size_t size) {
  _spi->beginTransaction(SPISettings(MCP3x6x_SPI_SPEED, MCP3x6x_SPI_ORDER, MCP3x6x_SPI_MODE));
  noInterrupts();
  digitalWrite(_pinCS, LOW);
  _status.raw = _spi->transfer(addr);
#ifdef ARDUINO_ARCH_ESP32
  _spi->writeBytes(data, size);
#else
  _spi->transfer(data, size);
#endif
  digitalWrite(_pinCS, HIGH);
  interrupts();
  _spi->endTransaction();

  return _status;
}
McMillanFlowProducts commented 5 months ago

Just changed the _transfer function, still getting a reply of zero. I am LeKrakenTheCode, I just ended up creating the issue under the wrong account and never changed it.

*** I have removed a println from the analogread function, i am now getting a constant value of 130 (raw adc value) regardless of what i am reading in the mux. i have updated code on github. image

image

image