I2C slave: enable matching any configured slave address

[deltaford]

I2C slave: Enable multi slave address onReceive/onRequest handling.

Currently the I2C slave onRequest/onReceive events are only triggered if matching OwnAddress1.
With this proposed change the events are triggered for any configured, enabled and matching slave address.

[ABOSTM]

Hi @deltaford,
I am not sure about the interest of this change:
On STM32, in I2C slave mode, I2C hardware will filter on I2C Addresses configured in the hardware registers.
2 addresses are configurable (See reference manual I2C_OAR1 and I2C_OAR2).
But in stm32duino, only 1 address is configured, OwnAddress1:

Arduino_Core_STM32/libraries/Wire/src/utility/twi.c

Line 764 in bf0b3e3

handle->Init.OwnAddress1 = ownAddress;

Note that OwnAddress2 is systematically set to 0

So AFAICS, this change will not help enabling multi slave address.
Did you make some testing with your patch? does it change the behavior ?
(does it really answer to several I2C addresses ?)

That said, the software check against OwnAddress1 seems useless to me, because I2C hardware should already check that.
So I would be in favor of removing the check onn AddrMatchCode.

[deltaford]

Hi @ABOSTM
This change do not provide stm32duino capability to configure multiple i2c slave addresses, ideal that should also be implemented instead of OA2 being zeroed out as it is now. But that was too big of a bite for myself to implement.
Configuring multiple addresses is fairly easy from user code by modifying the I2C_OAR2 register.
My proposed change enable stm32duino to handle events (onReceive/onRequest) for multiple slave addresses on same i2c.
Also as you mentioned, the AddrMatchCode check is redundant since the code would not execute unless i2c hardware already matched configured addresses. Removing it completely would optimize the code and enable multi address handling at the same time.
With OA1, OA2 and OA2MSK i2c can be configured to match 1, 2 or more up to all possible addresses (except reserved).
With this change it is possible to handle slave events on multiple addresses and also different handling depending on what address matched.
I have tested the patch and I'm using it in a project where the functionality was required.

Here is also a small example

// STM32 Multi address slave example - I2C1 (Slave: #2, #3), I2C2 (Master), externally connected and with external pullups

#include <Wire.h>
TwoWire Wire1(PB7, PB6);
TwoWire Wire2(PB11, PB10);

uint8_t I2C1_ADDR1 = 2, I2C1_ADDR2 = 3, tmpin = 0, OA2MASK = 0 ; 

void setup(){
  Serial2.begin(115200);           
  delay(50);  
  Wire1.begin(I2C1_ADDR1);         
  Wire1.onRequest(requestEvent); 
  Wire1.onReceive(receiveEvent); 
  MODIFY_REG(I2C1->OAR2, I2C_OAR2_OA2 , (I2C1_ADDR2 << 1) | (OA2MASK << 10));  // Configure OwnAddress2 
  SET_BIT(I2C1->OAR2, I2C_OAR2_OA2EN);      // Dual addressing mode enable
  Wire2.begin();
}
void loop(){
  delay(1000);
  Wire2.beginTransmission(I2C1_ADDR1);
  Wire2.write(7);
  delay(1000);
  Wire2.endTransmission();  
  Wire2.beginTransmission(I2C1_ADDR2);
  Wire2.write(8);
  Wire2.endTransmission();  
  tmpin = Wire2.requestFrom(I2C1_ADDR1,(uint8_t) 1);
  tmpin = Wire2.requestFrom(I2C1_ADDR2,(uint8_t) 1);  
}
void receiveEvent(int howMany){
  Serial2.print("Slave receive. AddrMatch: "); Serial2.println((I2C1->ISR & I2C_ISR_ADDCODE) >> 17); ;
  int x = Wire1.read();        // receive byte as an integer
}
void requestEvent(){
  Serial2.print("Slave request. AddrMatch: "); Serial2.println((I2C1->ISR & I2C_ISR_ADDCODE) >> 17); 
  Wire1.write(4);  // respond 
}

[ABOSTM]

Ok, thanks for your detailed explanation.
So would you mind updating your PR to fully remove the check ?

[deltaford]

It's now updated and the check is removed.

[ABOSTM]

@deltaford
Thanks for the update, there is an astyle issue about indentation (see CI error)
Could you fix it please.
If needed: https://github.com/stm32duino/wiki/wiki/Astyle

[deltaford]

Should be fixed now

[ABOSTM]

LGTM