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

Function: _getValue(uint32_t raw) wrong return #7

Closed FelixWT closed 1 year ago

FelixWT commented 1 year ago

Hi, it's me again

I found out while the value should be negative number returns a very large positive number when I'm in data_format::SGN_DATA (int24)mode. For example, when the ADCDATA REGISTER have value FFFE2F,_getValue(raw) will return 16776751 while it should return -465;

The problem happend here: https://github.com/nerdyscout/Arduino_MCP3x6x_Library/blob/6cce10135956cceaa817b629f27d1a70c646e8a2/lib/MCP3x6x/MCP3x6x.cpp#L269-L272

I can't figure out what when wrong so I modified it like this:

case (data_format::SGN_DATA):  
  if(bitRead(raw, 23)){
    return (((~raw+1) & 0x7FFFFF)*-1);  // OR return((0x1000000 - raw & 0x7FFFFF)*-1);
  }else{
    return raw & 0x7FFFFF;
  }

if MSB==1(Negative Number), turn it to nergative number by two's complement, but we don't need to consider bytes greater then 22(0~22). So we & 0x007FFFFFF, but we still needs the negative sign so *-1.

I think there is some way to simplify this but this will work for me.



side note: I decided to write the whole register at once in the begin function like this

void MCP3x6x::setConfig0(){
  settings.config0.vref_sel=1;
  settings.config0.cfg0=1;
  settings.config0.clk=clk_sel::INTERN;
  settings.config0.bias=cs_sel::BIAS_0UA;
  settings.config0.adc=adc_mode::STANDBY;
  _status = write(settings.config0);
}
void MCP3x6x::setConfig1(){
  settings.config1.pre=pre::MCLK_0;
  settings.config1.osr=osr::OSR_256;
  _status = write(settings.config1);
}
void MCP3x6x::setConfig2(){
  settings.config2.boost=boost::BOOST_1;
  settings.config2.gain=gain::GAIN_32;
  settings.config2.az_mu=0;
  settings.config2.az_ref=1;
  settings.config2.reserved=1;
  _status = write(settings.config2);
}
void MCP3x6x::setConfig3(){
  settings.config3.conv_mode=conv_mode::CONTINUOUS;
  settings.config3.data_format=data_format::SGN_DATA;
  settings.config3.crc_format=0;
  settings.config3.en_crccom=0;
  settings.config3.en_offcal=1;
  settings.config3.en_gaincal=1;
  _status = write(settings.config3);
}
bool MCP3x6x::begin(uint16_t channelmask, float vref) {
  pinMode(_pinCS, OUTPUT);
  digitalWrite(_pinCS, HIGH);

  _spi->begin(_pinCLK,_pinMISO,_pinMOSI,_pinCS);
  _status = reset();

  setConfig0();
  setConfig1();
  setConfig2();
  setConfig3();
  return true;
}

to check if the counfigurations actually apply to MCP3X6X

void MCP3x6x:: readBuffer(){  
  uint8_t buffer[1];
  _transfer(buffer, MCP3x6x_CMD_IREAD | MCP3x6x_ADR_CONFIG0);
  Serial.printf("CONFIG0: %02X\n",buffer[0]);

  _transfer(buffer, MCP3x6x_CMD_IREAD | MCP3x6x_ADR_CONFIG1);
  Serial.printf("CONFIG1: %02X\n",buffer[0]);

  _transfer(buffer, MCP3x6x_CMD_IREAD | MCP3x6x_ADR_CONFIG2);
  Serial.printf("CONFIG2: %02X\n",buffer[0]);

  _transfer(buffer, MCP3x6x_CMD_IREAD | MCP3x6x_ADR_CONFIG3);
  Serial.printf("CONFIG3: %02X\n",buffer[0]);
}

Best,

Mirageofmage commented 1 year ago

This is intended functionality. If you look at the definition of the SGN_DATA of the data_format class it states that it does not allow overrange and the ADC code is locked to (0xFFFFFF or 0x800000) (Positive numbers only)

https://github.com/nerdyscout/Arduino_MCP3x6x_Library/blob/6cce10135956cceaa817b629f27d1a70c646e8a2/lib/MCP3x6x/MCP3x6x.h#L261-L262

It looks like the mode you should have been using is SGNEXT_DATA

https://github.com/nerdyscout/Arduino_MCP3x6x_Library/blob/6cce10135956cceaa817b629f27d1a70c646e8a2/lib/MCP3x6x/MCP3x6x.h#L257-L258

Where the LSB in the 4th byte is the sign

FelixWT commented 1 year ago

I might misunderstanding on what datasheet discribes, but what I see so far from data sheet :

image

As above, it should at least support positive integer number from 0x000001 ~ 0x7FFFFF (1 ~ 8388607) and negative number 0xFFFFFF ~ 0x800000 (-1 ~ -8388608).

And also what do you mean by overrange ? The numbers that over pass 0xFFFFFF ?

Mirageofmage commented 1 year ago

Table 5-7 of the reference manual just refers the output codes to expected input voltages. According to the reference manual, by default the ADC will output a 24-bit number with MSB reserved for twos complement. Arduino/C does not have a type for 24-bit numbers which is probably why you're seeing a larger number instead of the negative (if you're storing the number as an int32_t). This is why I recommended using SGNEXT_DATA as it makes calculation of two's complement much easier (Only check the value of the highest byte).

However, I'm confused on why the ADC data should return -465, are you supplying a -Vref to the chip (below ground)?

If you're storing the value in an int32_t, consider using the SGN_DATA_ZERO mode and dropping/ignoring the last byte

https://github.com/nerdyscout/Arduino_MCP3x6x_Library/blob/6cce10135956cceaa817b629f27d1a70c646e8a2/lib/MCP3x6x/MCP3x6x.h#L259-L260

FelixWT commented 1 year ago

However, I'm confused on why the ADC data should return -465, are you supplying a -Vref to the chip (below ground)?

Wait, it's impossible to supply with negative voltage right? I thought -Vref in the table meant ,compare to SIG+(Loadcell +) of my ADC DEVICE, SIG-(Loadcell -) have larger value than SIG+;

image

FelixWT commented 1 year ago

This is why I recommended using SGNEXT_DATA as it makes calculation of two's complement much easier (Only check the value of the highest byte).

True, but since it require more buffer space and more bit to transfer,doesn't change it to 32bit mode will impact on sampling rate?

Mirageofmage commented 1 year ago

However, I'm confused on why the ADC data should return -465, are you supplying a -Vref to the chip (below ground)?

Wait, it's impossible to supply with negative voltage right? I thought -Vref in the table meant ,compare to SIG+(Loadcell +) of my ADC DEVICE, SIG-(Loadcell -) have larger value than SIG+;

image

Oh yes, this is true if you are making a differential and not a single ended measurement.

The impact on sampling rate depends on how fast you're running the SPI bus. Eventually it will hit a limit, but changing to a 32-bit mode depends on how much data you're transferring over the bus. I've gotten about 1000Hz from a single input off of the ADC, I'm assuming you won't need that much bandwidth for a load cell but I could be wrong.

nerdyscout commented 1 year ago

afaik the ADC sampling is driven by MCLK and therefore independent from SPI speed. Biggest influence on sample rate is done by timer and oversampling settings. Of course, reading data slower than converting it you will lose values, this is why ADCDATA register is double buffered to ensure you never read corrupted data. for high datarate I would suggest to connect pin IRQ as to an interrupt pin and read in ISR

FelixWT commented 1 year ago

Biggest influence on sample rate is done by timer and oversampling settings

So from what you said, the influence change from 24 bit to 32 bit is too tiny that we can ignore it?

for high datarate I would suggest to connect pin IRQ as to an interrupt pin and read in ISR

Sounds great! I'll try this if I need to.

Back to the topic ,it seems the getValue function is specific build for single input, isn't really a bug, so I''ll close this for now.