Die Temperature Module takes a considerable amount of time to Query

Tip / Sign in to post questions, reply, level up, and achieve exciting badges. Know more

cross mob
niprc_2224616
Level 2
Level 2
Welcome!

Hi,

I was running into performance issues on one of my slower ISRs (1ms).  I started commenting out some of the newer status checks and I noticed that when the DieTemp block was disabled, my problems disappeared.

All I did was call

// read out temperature in Celsius

temp = DieTemp_Query(&device.MCU_temp);

if(temp == CYRET_SUCCESS)

{

    DieTemp_GetTemp(&device.MCU_temp);

}

else if(temp != CYRET_STARTED)

{

    DieTemp_Start();

}

for reference "device" is a data structure for telemetry and "MCU_temp" is a uint16 within that structure.

That block of code was taking on average about 1.8ms.  Why is this so slow and is there an efficient way to non-block poll the Die Temperature in real time? 

Let me know if I did something silly as always...

Nick

0 Likes
1 Solution

Len,

Thank you for the quick answer.

Are you calling the Die Temp APIs from ISR?  I learned very long ago from an expert programmer to only perform the bare minimum in an ISR and get out.  An ISR works best if it can operate in less than 10us if possible.

I agree with you.  I am currently measuring performance with GPIOs and our max ISR frame time is 6us (outside of the die temp check).  To give you background, the system is a motor controller with two ISRs (one 20kHz for current PID and a second at 1kHz for cascaded velocity PID) and a main loop (RS-422 with 10ms command response and USB with 50ms command response; both packets run over 100kb with 10byte packets).  I do my best to keep each frame under 5us and utilize fixed point math in most places to avoid floating point delays on the Cortex M3.  I have spent the better part of 15 years in VHDL design, but there are nuances to firmware development that still get me..  I find that running status checks in the slower ISR is a good place so that my protocol section in main can run uninterrupted.

DieTemp_Start() and DieTemp_Query() are used for the non-blocking.  I recommend that in the ISR, you set a flag for your main() loop (or task scheduler) to be signalled to get the Die_Temp.  There you can perform other functions while waiting for the Die_Temp APIs to complete.

This is a good work around.  While I play around with a modification to the API, I am doing this.  Thank you.  I am not going to use this as a final solution; however, since this can cut my RS-422 packets up and I like to synchronize my RS-422 with a 2ms timeout.  The DieTemp_GetTemp() takes far too long for those checks I am running.

I have been looking at the API and I am creating a substitute method for DieTemp_GetTempT(). The problem I see with DieTemp_GetTempT() is that there is an issue on the PSoC5LP where there must be two reads prior to seeing and accurate reading.  To work around this, the devs added a double read and wait..

/*******************************************************************************

* Function Name: DieTemp_GetTemp

********************************************************************************

*

* Summary:

*  Sets up the command to get the temperature and blocks until finished. After

*  DieTemp_MAX_WAIT ticks the function will return even if the

*  SPC has not finished.

*

* Parameters:

*  temperature: Address to store the two byte temperature value.

*

* Return:

*  CYRET_SUCCESS if the command was completed sucessfuly.

*  Status code from DieTemp_Start or DieTemp_Query

*

*******************************************************************************/

static cystatus DieTemp_GetTempT(int16 * temperature)

{

    uint16 us;

    cystatus status;

    /* Start the temp reading */

    status = DieTemp_Start();

    if(status == CYRET_STARTED)

    {

        /**************************************************************************

        * Wait for SPC to finish temperature reading. If state will change and SPC

        * will finish - break cycle.

        * DieTemp_MAX_WAIT is maximum time in ms to wait for SPC.

        **************************************************************************/

        for (us=(DieTemp_MAX_WAIT*1000u); us>0u; us-=10u)

        {

            if((CYRET_STARTED != status)&&(CY_SPC_IDLE))

            {

                break;

            }

            else if(CYRET_STARTED == status)

            {

                status = DieTemp_Query(temperature);

            }

            else

            {

                /* SPC has not finished reading or isn't idle */

            }

            CyDelayUs(10u);

        }

    }

    return status;

}

cystatus DieTemp_GetTemp(int16 * temperature)

{

    cystatus status;

    uint8 count = 2u;

    while (count != 0u)

    {

        status = DieTemp_GetTempT(temperature);

        if (status != CYRET_SUCCESS)

        {

            break;

        }

        count--;

    }

    return status;

}

There is a pretty nasty

for (us=(DieTemp_MAX_WAIT*1000u); us>0u; us-=10u)

I assume since my ISR is 1ms, I can just break this method in two and only sample the second read without adding a wait in between.  I am adding a few updates to the firmware so I will test this and post a solution if I find something with a little better performance.

I will test this in a bit and let you know what I find.

Thank you,

Nick

View solution in original post

0 Likes
4 Replies
Len_CONSULTRON
Level 9
Level 9
Beta tester 500 solutions authored 1000 replies posted

Nick,

Are you calling the Die Temp APIs from ISR?  I learned very long ago from an expert programmer to only perform the bare minimum in an ISR and get out.  An ISR works best if it can operate in less than 10us if possible.

About your issue:  I believe part of your issue is that you are performing the DieTemp_GetTemp().  This is a blocking function. It functionally does the equivalent of DieTemp_Start() along with at least two calls to DieTemp_Query() in a tight loop.

DieTemp_Start() and DieTemp_Query() are used for the non-blocking.  I recommend that in the ISR, you set a flag for your main() loop (or task scheduler) to be signalled to get the Die_Temp.  There you can perform other functions while waiting for the Die_Temp APIs to complete.

Len

Len
"Engineering is an Art. The Art of Compromise."
0 Likes

Len,

Thank you for the quick answer.

Are you calling the Die Temp APIs from ISR?  I learned very long ago from an expert programmer to only perform the bare minimum in an ISR and get out.  An ISR works best if it can operate in less than 10us if possible.

I agree with you.  I am currently measuring performance with GPIOs and our max ISR frame time is 6us (outside of the die temp check).  To give you background, the system is a motor controller with two ISRs (one 20kHz for current PID and a second at 1kHz for cascaded velocity PID) and a main loop (RS-422 with 10ms command response and USB with 50ms command response; both packets run over 100kb with 10byte packets).  I do my best to keep each frame under 5us and utilize fixed point math in most places to avoid floating point delays on the Cortex M3.  I have spent the better part of 15 years in VHDL design, but there are nuances to firmware development that still get me..  I find that running status checks in the slower ISR is a good place so that my protocol section in main can run uninterrupted.

DieTemp_Start() and DieTemp_Query() are used for the non-blocking.  I recommend that in the ISR, you set a flag for your main() loop (or task scheduler) to be signalled to get the Die_Temp.  There you can perform other functions while waiting for the Die_Temp APIs to complete.

This is a good work around.  While I play around with a modification to the API, I am doing this.  Thank you.  I am not going to use this as a final solution; however, since this can cut my RS-422 packets up and I like to synchronize my RS-422 with a 2ms timeout.  The DieTemp_GetTemp() takes far too long for those checks I am running.

I have been looking at the API and I am creating a substitute method for DieTemp_GetTempT(). The problem I see with DieTemp_GetTempT() is that there is an issue on the PSoC5LP where there must be two reads prior to seeing and accurate reading.  To work around this, the devs added a double read and wait..

/*******************************************************************************

* Function Name: DieTemp_GetTemp

********************************************************************************

*

* Summary:

*  Sets up the command to get the temperature and blocks until finished. After

*  DieTemp_MAX_WAIT ticks the function will return even if the

*  SPC has not finished.

*

* Parameters:

*  temperature: Address to store the two byte temperature value.

*

* Return:

*  CYRET_SUCCESS if the command was completed sucessfuly.

*  Status code from DieTemp_Start or DieTemp_Query

*

*******************************************************************************/

static cystatus DieTemp_GetTempT(int16 * temperature)

{

    uint16 us;

    cystatus status;

    /* Start the temp reading */

    status = DieTemp_Start();

    if(status == CYRET_STARTED)

    {

        /**************************************************************************

        * Wait for SPC to finish temperature reading. If state will change and SPC

        * will finish - break cycle.

        * DieTemp_MAX_WAIT is maximum time in ms to wait for SPC.

        **************************************************************************/

        for (us=(DieTemp_MAX_WAIT*1000u); us>0u; us-=10u)

        {

            if((CYRET_STARTED != status)&&(CY_SPC_IDLE))

            {

                break;

            }

            else if(CYRET_STARTED == status)

            {

                status = DieTemp_Query(temperature);

            }

            else

            {

                /* SPC has not finished reading or isn't idle */

            }

            CyDelayUs(10u);

        }

    }

    return status;

}

cystatus DieTemp_GetTemp(int16 * temperature)

{

    cystatus status;

    uint8 count = 2u;

    while (count != 0u)

    {

        status = DieTemp_GetTempT(temperature);

        if (status != CYRET_SUCCESS)

        {

            break;

        }

        count--;

    }

    return status;

}

There is a pretty nasty

for (us=(DieTemp_MAX_WAIT*1000u); us>0u; us-=10u)

I assume since my ISR is 1ms, I can just break this method in two and only sample the second read without adding a wait in between.  I am adding a few updates to the firmware so I will test this and post a solution if I find something with a little better performance.

I will test this in a bit and let you know what I find.

Thank you,

Nick

0 Likes

I added a non-blocking replacement for the DieTemp API.

void getTemp_API_byp(DEVICE_DATA* device)

{

    int16 trash;

   

    if(device->temp_status == CYRET_STARTED)

    {

        if(device->temp_valid)

        {

            device->temp_status = DieTemp_Query(&device->MCU_temp);

            device->temp_valid = 0;

        }

        else

        {

            device->temp_status = DieTemp_Query(&trash);

            device->temp_valid++;

        }

    }  

    else

    {

        device->temp_status = DieTemp_Start();

    }

   

    return;   

}

It's a pretty small function that appears to be working without a 1-2ms block (a few us now).  The only concern I have is related to my use of DieTemp_Query.  I see an SPC lock in here.  If I include this in an ISR call, there will be a 5ms (round trip) delay between updated calls.  Does anyone in the forums know if this will have an effect on performance?  I browsed the TRM for the PSoC5LP and the information around the SPC is not 100% clear w.r.t. processor behavior when there are long locks on the SPC.

Nick

0 Likes

Nick,

Be careful placing calls to functions that block or have indeterminate time-locks such as the SPC lock in ISRs

In principle, only perform the MINIMUM very time limited operations in an ISR.   If additional time-intensive operations are needed, set a signal (boolean flag) in the ISR that is then processed by the main() loop in your application.

This prevents temporary code lock up conditions where the code is stuck at interrupt level for excessive amounts of time.  It also reduces the chance of stack overruns since the ISR uses additional stack.

Len

Len
"Engineering is an Art. The Art of Compromise."
0 Likes