Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed memory on Wire instance #2196

Merged
merged 1 commit into from
Nov 22, 2023
Merged

Fixed memory on Wire instance #2196

merged 1 commit into from
Nov 22, 2023

Conversation

gbr1
Copy link
Contributor

@gbr1 gbr1 commented Nov 21, 2023

Fixing memory cleanup on Wire instance

Summary

The constructor doesn't cleanup memory for a wire instance.

Issues could happen, such as wrong initialization.

This PR fixes/implements the following bugs/features

  • Issue on locked/bad allocated i2c on turning on

Example of bad behaviour using a STM32F411 and an APDS9960 sensor:

#include "Arduino_APDS9960.h"

APDS9960 * apds;
TwoWire * wire;

void setup() {
  Serial.begin(115200);

  wire=new TwoWire(PB7,PB8);
  wire->begin();
  apds = new APDS9960(*wire,PC10);
  apds->begin();
}

void loop() {
  if (apds->proximityAvailable()) {
    int value = apds->readProximity();
    Serial.println(value);
  }
}

The bad behaviour is that after turn off and on the board, wire locks.
Note: it works if you go from boot mode to running mode

This pull request solve this issue adding the following line in TwoWire constructors.

memset((void*)&_i2c, 0, sizeof(_i2c));

@gbr1
Copy link
Contributor Author

gbr1 commented Nov 21, 2023

cc @facchinm

@fpistm fpistm added this to In progress in STM32 core based on ST HAL via automation Nov 21, 2023
@fpistm fpistm added the Fix label Nov 21, 2023
@fpistm fpistm added this to the 2.7.1 milestone Nov 21, 2023
@fpistm fpistm self-requested a review November 21, 2023 14:59
@fpistm
Copy link
Member

fpistm commented Nov 21, 2023

Hi @gbr1
Thanks for this PR.
If you can update with the astyle fixed, please.

The constructor doesn't cleanup memory for a wire instance.
Issues could happen, such as wrong initialization during boot

Signed-off-by: Giovanni Bruno <giovannididio.bruno@gmail.com>
Copy link
Member

@fpistm fpistm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
Thanks @gbr1

STM32 core based on ST HAL automation moved this from In progress to Reviewer approved Nov 22, 2023
@fpistm fpistm merged commit 61a41ec into stm32duino:main Nov 22, 2023
22 checks passed
STM32 core based on ST HAL automation moved this from Reviewer approved to Done Nov 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants