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 leaks in rainmaker examples #7965

Merged
merged 5 commits into from
Mar 31, 2023

Conversation

sanketwadekar
Copy link
Contributor

Description of Change

This PR will fix the memory leaks (which existed for a long time but got exposed in the recent release) in Rainmaker examples and improves some error logs in the Rainmaker library.

NOTE: The actual memory allocation failures will be fixed in the Rainmaker C library.

Tests scenarios

I have tested this PR on esp32 wrover kit.

Related links

Related to #7893

@sanketwadekar
Copy link
Contributor Author

@shahpiyushv @vikramdattu Please take a look

@VojtechBartoska VojtechBartoska added the Area: Rainmaker Issue is related to ESP Rainmaker. label Mar 15, 2023
@VojtechBartoska VojtechBartoska added this to the 2.0.8 milestone Mar 15, 2023
Copy link

@vikramdattu vikramdattu left a comment

Choose a reason for hiding this comment

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

Minor cosmetic issues. LGTM otherwise.

my_device->updateAndReportParam(ESP_RMAKER_DEF_POWER_NAME, dimmer_state);
}
(dimmer_state == false) ? digitalWrite(gpio_dimmer, LOW) : digitalWrite(gpio_dimmer, HIGH);
}

Choose a reason for hiding this comment

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

Please follow same styling across the file

Choose a reason for hiding this comment

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

We are losing time with style issues when an automatic script with clang formatter could keep all PR at same pattern.

Choose a reason for hiding this comment

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

We have a script in IDF which uses astyle programme to achieve this: here. However this has to be run manually for this repo.

If you have a change for using automated clang formatter, would you please raise a PR for the same?

Choose a reason for hiding this comment

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

I'm not an expert in this subject otherwise I could make a PR. Actually I'm using uncrustify in VSCode only.

Anyway there's an action with example for artistic style here:
https://github.com/marketplace/actions/artistic-style

@@ -111,17 +113,19 @@ void loop()

if ((endTime - startTime) > 10000) {
// If key pressed for more than 10secs, reset all

Choose a reason for hiding this comment

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

Please follow consistent styling.

@me-no-dev me-no-dev merged commit c0737f5 into espressif:master Mar 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Rainmaker Issue is related to ESP Rainmaker.
Projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants