0
\$\begingroup\$

Hi guys may I ask for your inputs on the I2C communication. I am using STM32F103C8 MCU with 2 slaves on the I2C bus. 1st slave is PCF8574 IO expander and 2nd slave is MCP4725 DAC. I'm using CubeMX to generate my code.

What I wanted to do is use 2 buttons. The first button is connected to STM32 and when toggled this enables/disables one of the PCF8574 IO. The second button connected to the STM32 is used to enable/disable the MCP4725 DAC. Both slaves are connected to the same I2C line.

Basically if I put both slaves together and place them in the while(1) loop everything works fine. However when I use the callback the 2 slaves tend to work intermittently and sometimes hang.

MX_GPIO_Init();
MX_DMA_Init();
MX_I2C1_Init();  
/* USER CODE BEGIN 2 */

HAL_I2C_Master_Transmit_IT(&hi2c1, 0xC0, buffer_I2C, 3);                            
HAL_I2C_Master_Transmit_IT(&hi2c1, 0x40, buffer_PCF8574Write, 1);   
/* USER CODE END 2 */   
 
//call Master transfer complete callback

void HAL_I2C_MasterTxCpltCallback(I2C_HandleTypeDef *hi2c)                  
{
    
//define the pin combinations for the 2 buttons
    
    if((HAL_GPIO_ReadPin(BUTTON1_LED_GPIO_Port, BUTTON1_LED_Pin)) &&(!HAL_GPIO_ReadPin(BUTTON2_LED_GPIO_Port, BUTTON2_LED_Pin)))
    {
        buttonstate = 0xFF;     //PCF8574 out only
    }
    else if((!HAL_GPIO_ReadPin(BUTTON1_LED_GPIO_Port, BUTTON1_LED_Pin)) && (HAL_GPIO_ReadPin(BUTTON2_LED_GPIO_Port, BUTTON2_LED_Pin)))
    {
        buttonstate = 0xF0;     //MCP4725 out only
    }   
    else if((HAL_GPIO_ReadPin(BUTTON1_LED_GPIO_Port, BUTTON1_LED_Pin)) && (HAL_GPIO_ReadPin(BUTTON2_LED_GPIO_Port, BUTTON2_LED_Pin)))
    {
        buttonstate = 0x0F;     //PCF8574 and MCP4725  out
    }   
    else if((!HAL_GPIO_ReadPin(BUTTON1_LED_GPIO_Port, BUTTON1_LED_Pin)) && (!HAL_GPIO_ReadPin(BUTTON2_LED_GPIO_Port, BUTTON2_LED_Pin)))
    {
        buttonstate = 0x00;     //no PCF8574  and MCP4725  out
    }   

    
    
    switch(buttonstate)
    {
            //write to slave address 0x40 of PCF8574.  Configured active low.
        case 0xFF:
            buffer_PCF8574Write[0] = 0xFE;
            HAL_I2C_Master_Transmit_IT(&hi2c1, 0x40, buffer_PCF8574Write, 1);
            
            //write to slave address 0xC0 of MCP4725
            buffer_I2C[0] = 0x40;                                                                                   
            buffer_I2C[1] = 0x00;
            buffer_I2C[2] = 0x00;
            HAL_I2C_Master_Transmit_IT(&hi2c1, 0xC0, buffer_I2C, 3);
            break;
            
            //write to slave address 0xC0 of MCP4725
        case 0xF0:
            buffer_I2C[0] = 0x40;                                                                                   
            buffer_I2C[1] = 0xFF;
            buffer_I2C[2] = 0xF0;
            HAL_I2C_Master_Transmit_IT(&hi2c1, 0xC0, buffer_I2C, 3);    
            
            //write to slave address 0x40 of PCF8574.  Configured active low.
            buffer_PCF8574Write[0] = 0xFF;  
            HAL_I2C_Master_Transmit_IT(&hi2c1, 0x40, buffer_PCF8574Write, 1);
            break;
            
            
            //write to slave address 0x40 of PCF8574.  Configured active low.
        case 0x0F:
            buffer_PCF8574Write[0] = 0xFE;  
            HAL_I2C_Master_Transmit_IT(&hi2c1, 0x40, buffer_PCF8574Write, 1);
            
            //write to slave address 0xC0 of MCP4725
            buffer_I2C[0] = 0x40;                                                                                   
            buffer_I2C[1] = 0xFF;
            buffer_I2C[2] = 0xF0;
            HAL_I2C_Master_Transmit_IT(&hi2c1, 0xC0, buffer_I2C, 3);
            break;
            
            //write to slave address 0x40 of PCF8574.  Configured active low.
        case 0x00:
            buffer_PCF8574Write[0] = 0xFF;  
            HAL_I2C_Master_Transmit_IT(&hi2c1, 0x40, buffer_PCF8574Write, 1);
            
            //write to slave address 0xC0 of MCP4725
            buffer_I2C[0] = 0x40;                                                                                   
            buffer_I2C[1] = 0x00;
            buffer_I2C[2] = 0x00;
            HAL_I2C_Master_Transmit_IT(&hi2c1, 0xC0, buffer_I2C, 3);
            break;
            
        default:
            break;
    }
}
\$\endgroup\$
3
  • 1
    \$\begingroup\$ Please format the code so it is actually readable. But you can't start another interrupt based transaction while one is already in progress. \$\endgroup\$ – Justme Jul 31 '20 at 12:36
  • \$\begingroup\$ Inside your callback you're making multiple calls to HAL_I2C_Master_Transmit_IT. So you start a transmit on the I2C, and then before it's completed you start another one. You can't stack them up like that - you have to wait for one to finish before you start the next one. \$\endgroup\$ – brhans Jul 31 '20 at 13:41
  • \$\begingroup\$ The simplest (but not necessarily the best) way to do what you're trying to do is to move most of that code in the I2C callback into your main loop and use the blocking HAL_I2C_Master_Transmit instead. Doing what you're trying to do inside the callback (which is really part of the I2C interrupt handler) will require you to implement a small state machine so that you only attempt a single HAL_I2C_Master_Transmit_IT for each callback instance. \$\endgroup\$ – brhans Jul 31 '20 at 13:44
2
\$\begingroup\$

I see two possible problems:

1. When should you test for the button state?

Most code is in the I2C completion callback handler. So the buttons are only tested for their state if an I2C transaction has completed. The reason it works at all is that you are constantly sending I2C commands to the sensors.

Why are you using the completion handler at all? Turning the sensor on or off is basically a fire-and-forget operation. There are no further actions if the command has completed.

I guess the best solution is to move the code back into the endless loop.

2. Bouncing buttons and multiple calls of HAL_GPIO_ReadPin

A typical button bounces, i.e. it will quickly go on and off several times when pressed. Your code additionally combines it with multiple calls of HAL_GPIO_ReadPin for the same button. The result can be that these calls return different values for the same button as the button might have changed in the mean time. This might explain the strange behavior.

Anyhow, the code is overly complicated. Button 1 and 2 are completed independent. There is no reason for ever mixing the state of these.

Why not do something like this:

GPIO_PinState button_1_state = GPIO_PIN_RESET;
GPIO_PinState button_2_state = GPIO_PIN_RESET;

...

while (1) {
    GPIO_PinState new_state;

    // Test button 1
    new_state = HAL_GPIO_ReadPin(BUTTON1_LED_GPIO_Port, BUTTON1_LED_Pin);
    if (new_state != button_1_state) {
        if (new_state) {
            // turn sensor 1 on
        } else {
            // turn sensor 1 off
        }
        button_1_state = new_state;
    }

    // Test button 2
    new_state = HAL_GPIO_ReadPin(BUTTON2_LED_GPIO_Port, BUTTON2_LED_Pin);
    if (new_state != button_2_state) {
        if (new_state) {
            // turn sensor 2 on
        } else {
            // turn sensor 2 off
        }
        button_2_state = new_state;
    }


}

This still doesn't solve the bouncing. To deal with it, ignore a button's state changes if it has already been pressed during the last 100ms.

Update

3. Multiple calls of HAL_I2C_Master_Transmit_IT

As hansbr pointed out, only a single I2C transaction can be executing at a time. So if you call HAL_I2C_Master_Transmit_IT twice in short succession – which your code does in two places – the second call will have no effect (return an error) as there is already an ongoing I2C transction.

\$\endgroup\$
1
  • \$\begingroup\$ Thank you Sir for all the hints this gives me the solution. \$\endgroup\$ – newbie2020 Jul 31 '20 at 15:28

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.