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.

Sign up for GitHub

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

Bugfix: Fix compile error for ESP32 FSM ULP GPIO Example (IDFGH-9687) #11028

Closed
wants to merge 4 commits into from
Closed

Bugfix: Fix compile error for ESP32 FSM ULP GPIO Example (IDFGH-9687) #11028

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3eafff3
Change namespace variable to nvs_namespace to prevent expected unqual…
Kampi Mar 20, 2023
90849db
Undo wrong bugfix with type casting
Kampi Mar 22, 2023
0f93e52
Change namespace variable to nvs_namespace to prevent expected unqual…
Kampi Mar 20, 2023
c96429e
Merge branch 'v5.0_Bugfix_ULP_FSM_GPIO_Example' of github.com:Kampi/e…
Kampi Mar 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions examples/system/ulp_fsm/ulp/main/ulp_example_main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,27 +98,27 @@ static void init_ulp_program(void)

static void update_pulse_count(void)
{
const char* namespace = "plusecnt";
const char* nvs_namespace = "plusecnt";
const char* count_key = "count";

ESP_ERROR_CHECK( nvs_flash_init() );
nvs_handle_t handle;
ESP_ERROR_CHECK( nvs_open(namespace, NVS_READWRITE, &handle));
ESP_ERROR_CHECK( nvs_open(nvs_namespace, NVS_READWRITE, &handle));
uint32_t pulse_count = 0;
esp_err_t err = nvs_get_u32(handle, count_key, &pulse_count);
assert(err == ESP_OK || err == ESP_ERR_NVS_NOT_FOUND);
printf("Read pulse count from NVS: %5"PRIu32"\n", pulse_count);
Copy link
Contributor

Choose a reason for hiding this comment

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

@Kampi What is the reason for this change? The formatting macros have been introduced to fix warnings after switching between compilers for different platforms (e.g. Xtensa-gcc to RISCV-gcc). If it is because of the C++ compiler warning about invalid suffix on literal, please add the suggested change instead of changing the macro back to a hard-coded formatter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @0xjakob,

thanks for your suggestion. Your argument makes sense to me and I guess I had an error in my setup because I forgot (or accidentally removed) #include <inttypes.h>, which leads to a compilation error. I have modified the pull request and reverted this change.

Copy link
Contributor

Choose a reason for hiding this comment

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

@Kampi OK, thanks for the fix! Would you mind squashing the commits into one? Then we wouldn't have an intermediate commit that could potentially break builds on machines where the print formatters are different.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @0xjakob,
not sure if this was successful, because I´ve never done this before. Please check it. Otherwise I can create a new merge request.

Copy link
Contributor

Choose a reason for hiding this comment

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

@Kampi OK, let's leave it as it is then. We will amend the commits to squash them. The only difference is that the PR will be seen as closed instead of merged. The rest stays the same. In particular, your authorship of the commit in IDF will be retained.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @0xjakob
It's fine. Thanks!

printf("Read pulse count from NVS: %5d\n", (unsigned int)pulse_count);

/* ULP program counts signal edges, convert that to the number of pulses */
uint32_t pulse_count_from_ulp = (ulp_edge_count & UINT16_MAX) / 2;
/* In case of an odd number of edges, keep one until next time */
ulp_edge_count = ulp_edge_count % 2;
printf("Pulse count from ULP: %5"PRIu32"\n", pulse_count_from_ulp);
printf("Pulse count from ULP: %5d\n", (unsigned int)pulse_count_from_ulp);

/* Save the new pulse count to NVS */
pulse_count += pulse_count_from_ulp;
ESP_ERROR_CHECK(nvs_set_u32(handle, count_key, pulse_count));
ESP_ERROR_CHECK(nvs_commit(handle));
nvs_close(handle);
printf("Wrote updated pulse count to NVS: %5"PRIu32"\n", pulse_count);
printf("Wrote updated pulse count to NVS: %5d\n", (unsigned int)pulse_count);
}