MCC134 ADC Init - confusing code about daqhats HOT 8 CLOSED

Hello @jagauthier,

Measurement Computing does not provide support for the low level driver code you are asking about. As Linux is an open source architecture, you are welcome to poke around the source files for your own interests or at your own risk. I'm sure the Engineer(s) who developed this software had their reasons for this code. The examples work correctly is the proof.

Measurement Computing provides support for the top level API and examples. As you are interested in the MCC 134, please limit your questions to the C and Python examples such as single_value_read.c, single_value_read.py, etc.

from daqhats.

Hi @jeffreyg3

I just saw your reply because I've enable notification on the project as I'm a mcc172 user.

I don't think @jagauthier is looking for support, he's rather questioning some written code which, after looking at it, is indeed questionable. So I think it can be treated as a bug report.

I'm sure the Engineer(s) who developed this software had their reasons for this code. The examples work correctly is the proof.

does this mean your engineers do not make mistakes? Sure the example works but this does not mean there is not bug. If you only test with a device with the right ID, you do not expect the test to fail which should fail if a wrong device is used but which actually may not fail due to the mentioned behavior (unless me and @jagauthier are wrong).

from daqhats.

@jagauthier you are correct, the receive buffer parameter was omitted. It should have been:

_mcc134_spi_transfer(address, buffer, buffer, 4)

To write from and read back into the same buffer. I will update the code to correct that.

from daqhats.

@jeffreyg3 @bearsh @nwright-mcc

I was actually more reporting a bug than asking for support. Being dismissed out of hand was a little annoying using open source as the argument. This is the actual beauty of open source.

Anyway, @nwright-mcc there is more to it than that.

A few things going on this in section of code that are wrong and/or don't make sense. I will detail them now

Here's the code in question again

    // check ID and device ready
    buffer[0] = CMD_RREG | REG_ID;
    buffer[1] = 2 - 1;  // count - 1
    buffer[2] = CMD_NOP;
    buffer[3] = CMD_NOP;
    if ((result = _mcc134_spi_transfer(address, buffer, NULL, 4)) != 
        RESULT_SUCCESS)
    {
        _release_board_lock(address);
        return result;
    }
    
    if ((buffer[2] & 0x0E) != 0x000)
    {
        // incorrect ID
        _release_board_lock(address);
        return RESULT_INVALID_DEVICE;
    }
    if ((buffer[3] & 0x40) != 0x00)
    {
        _release_board_lock(address);
        return RESULT_BUSY;
    }

Starting from the top:

    // check ID and device ready
    buffer[0] = CMD_RREG | REG_ID;
    buffer[1] = 2 - 1;  // count - 1
    buffer[2] = CMD_NOP;
    buffer[3] = CMD_NOP;

According to the datasheet CMD_RREG second by is always 1.
I understand why the code here says 2-1, because CMD_WREG's second byte is length-1.
So this doesn't make sense here. Also, there is no need for buffer[3] at all.

The second portion:

if ((result = _mcc134_spi_transfer(address, buffer, NULL, 4)) != 
        RESULT_SUCCESS)

Needs to return data. So a simple fix for this is what @nwright-mcc posted with one addition. 3 bytes instead of 4.

if ((result = _mcc134_spi_transfer(address, buffer, buffer, 3)) != 
        RESULT_SUCCESS)

Next, the ID check is wrong.
The datasheet shows this information for the ID register:

The data that returns (on my board) is 0x08. (0000 1000) By masking this with 0x0E (000 1110) it checks against bits[3:1]. the check should be against bits [2:0]
So the line should not be

    if ((buffer[2] & 0x0E) != 0x000)

but instead

    if ((buffer[2] & 0x07) != 0x00)

(
zero is zero, but 0x000 also makes no sense at all and just makes the code harder to understand.

Lasly, since CMD_RREG only reads one byte, I mentioned buffer[3] is pointless.
So this code:

    if ((buffer[3] & 0x40) != 0x00)
    {
        _release_board_lock(address);
        return RESULT_BUSY;
    }

Should be removed.

So, the final, fixed, block of code should look like this:

    buffer[0] = CMD_RREG | REG_ID;
    buffer[1] = 1;
    buffer[2] = CMD_NOP;

    if ((result = _mcc134_spi_transfer(address, buffer, buffer, 3)) !=
        RESULT_SUCCESS)
    {
        _release_board_lock(address);
        return result;
    }
    if ((buffer[2] & 0x07) != 0x000)
    {
        // incorrect ID
        _release_board_lock(address);
        return RESULT_INVALID_DEVICE;
    }

from daqhats.

@jagauthier I think you may have an incorrect ADC data sheet. The second parameter for RREG is the number of registers to read minus 1:

I also read the status register into buffer[3] and check the nRDY bit to confirm the ADC is ready. You are correct that buffer[2] should be ANDed with 0x7 vs 0xE.

from daqhats.

Correct data sheet. Shortened reading. Oops! How are you reading the status register if you haven't asked for it.
The status register is 0x01 and it's never being requested in this code.

from daqhats.

The RREG command starts reading at the specified register address and returns subsequent registers. Specifying REG_ID as the start register and a count of 2 will return REG_ID in buffer[2] and REG_STATUS in buffer[3].

from daqhats.

Cool, I missed that bit (no pun intended?) Easy fix then!

from daqhats.