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

Usermod BME280: Fix compilation errors for ESP32-C3 (Fixes #4362) #4363

Open
wants to merge 32 commits into
base: main
Choose a base branch
from

Conversation

euphi
Copy link

@euphi euphi commented Dec 7, 2024

Fix for #4362:

At least for ESP32-C3 the call to the String constructor to convert numbers to String with a parameterized count of Decimals fails, because it is ambigious. --> explicitly cast the decimals parameters to unsigned long.

Alternative would be to directly use unsigned long instead of uint8_t.

netmindz and others added 30 commits November 15, 2024 20:27
Fixed point calculation for improved accuracy, dithering in debug builds only.
Averaging and optional multiplier can be set as compile flags, example for speed testing with long averaging and a 10x multiplier:

-D FPS_CALC_AVG=200
-D FPS_MULTIPLIER=10

The calculation resolution is limited (9.7bit fixed point) so values larger than 200 can hit resolution limit and get stuck before reaching the final value.

If WLED_DEBUG is defined, dithering is added to the returned value so sub-frame accuracy is possible in post-processingwithout enabling the multiplier.
bitshift was still set from testing, forgot to update
dithering is not really needed, the FPS_MULTIPLIER is a much better option.
Avoiding name collisions with the 'delay' function.
Don't generate a response if there's no HTTP request.

Fixes Aircoookie#4269
While not used by most bus types, it's not an optional parameter.
- previous fix worked but there was still an overflow after some time passed. there were still missing roll-overs apparently: reverting these two variables back to 16bit/8bit should fix it for good.
At least for ESP32-C3 the call to the String constructor to convert numbers to String with a parameterized count of Decimals fails, because it is ambigious. --> explicitly cast the decimals parameters to `unsigned long`.

Alternative would be to directly use `unsigned long` instead of `uint8_t`.
@DedeHai
Copy link
Collaborator

DedeHai commented Dec 9, 2024

thanks for fixing this.
did you test that it now compiles for all systems? i.e. ESP8266, ESP32, C3, S2, S3?

@softhack007 softhack007 added the rebase needed This PR needs to be re-based to the current development branch label Dec 11, 2024
@euphi
Copy link
Author

euphi commented Dec 11, 2024


Environment           Status    Duration
--------------------  --------  ------------
nodemcuv2             SUCCESS   00:00:36.675
nodemcuv2_compat      SUCCESS   00:00:38.667
nodemcuv2_160         SUCCESS   00:00:40.222
esp8266_2m            SUCCESS   00:00:44.633
esp8266_2m_compat     SUCCESS   00:00:44.453
esp8266_2m_160        SUCCESS   00:00:45.245
esp01_1m_full         SUCCESS   00:00:45.084
esp01_1m_full_compat  SUCCESS   00:00:44.778
esp01_1m_full_160     FAILED    00:00:41.062
esp32dev              SUCCESS   00:01:08.971
esp32_eth             SUCCESS   00:00:38.853
esp32_wrover          SUCCESS   00:01:05.252
esp32c3dev            SUCCESS   00:00:17.645
esp32s3dev_16MB_opi   SUCCESS   00:00:43.786
esp32s3dev_8MB_opi    SUCCESS   00:00:43.590
esp32s3_4M_qspi       SUCCESS   00:00:44.035
lolin_s2_mini         SUCCESS   00:00:56.890

esp01_1m_full_160 failed due to slightly too large binary size, so its unrelated.

@netmindz netmindz changed the base branch from 0_15_0 to main December 16, 2024 13:17
@netmindz netmindz changed the base branch from main to 0_15_0 December 16, 2024 13:17
@netmindz
Copy link
Collaborator

Sadly I can't just update this PR, it will need manual rebase

@euphi euphi changed the base branch from 0_15_0 to main December 20, 2024 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rebase needed This PR needs to be re-based to the current development branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants