search

CuriousScientist0 / ADS1256

An Arduino-compatible library for the ADS1256 24-bit analog-to-digital converter.
MIT License
26 stars 9 forks source link

function setPGA accidentally writes to all bits of ADCON #2

Closed DavidJRichards closed 7 months ago

DavidJRichards commented 9 months ago

There is a problem with the way the ADCON register is set in the setPGA function. the variable _ADCON is used before it is set. Also, the bit mask and set operations are flawed.

The demo program writes to the PGA registers but the corresponding read operation reads ther GPIO Control Register Also, the displayed values should be masked to 0b111 to show only the PGA bits.

As a quick fix I amended the InitialzieADC function by commenting one line and adding two more: (note CLK set to fCLKIN in this change) // writeRegister(ADCON_REG, B00000000); //ADCON - CLK: OFF, SDCS: OFF, PGA = 0 (+/- 5 V) _ADCON = B00100000; //ADCON - CLK: fCLKIN, SDCS: OFF, PGA = 0 (+/- 5 V) writeRegister(ADCON_REG, _ADCON);

And changing the setPGA function by commenting two lines and adding three more: // _ADCON = _ADCON | 0b00000111; //Change bit 2-0 to 1, leave rest unchanged // _ADCON = _ADCON & _PGA; //Combine the new _ADCON with the _PGA (changes bit 0-2)

define pga_mask 0b00000111

_ADCON &=~ pga_mask; //Change bit 2-0 to 1, leave rest unchanged 
_ADCON |= (_PGA & pga_mask); //Combine the new _ADCON with the _PGA (changes bit 0-2)

I discovered these problem whilst trying to enable fCLKOUT, the setPGA function was clobbering the setting. hth David.

CuriousScientist0 commented 8 months ago

Hi David! Thanks for spotting this error! I will soon handle this issue in an upcoming update.

CuriousScientist0 commented 7 months ago

Hi! The most recent update (2024 February 8) of the library should solve the issue.

I tested it by first updating the PGA to 3. I read it by reading the ADCON register, which returned me 3. Then I set the ADCON to 99 (0b01100011). Reading the PGA (reading ADCON, then masking the bits) still returned 3. Then I modified the PGA to 0. Then I read ADCON again and I got 96 (0b01100000) which is the correct value.

I actually did not treat the variables very professionally, and sometimes I did not update them when I should have. This caused the mess. I did some polishing on the code and modified several other functions to make sure that the code always uses the most up-to-date register values and it also keeps the variables up-to-date.

I hope it works on your side, too!