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

Enhancement/warning reduction #172

Merged
merged 2 commits into from
Jul 19, 2018
Merged

Enhancement/warning reduction #172

merged 2 commits into from
Jul 19, 2018

Conversation

Kameeno
Copy link
Contributor

@Kameeno Kameeno commented Jul 2, 2018

No description provided.

@Kameeno Kameeno requested review from 3rwww1, trap98, LeSuedois and Axe-R July 2, 2018 14:11
@Kameeno Kameeno force-pushed the enhancement/Warning-reduction branch from 8311d63 to d6d5db2 Compare July 2, 2018 15:14
// Make sure sensors had enough time to turn on.
// BME280 requires 2ms to start up.
delay(1000);
DEBUG_VAR("BME280 status after begin is:", envSensorStatus);
DEBUG_VAR("BME280 status after begin is:", this->envSensor.begin());
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I don't think you should call begin() twice, sorry... What was the warning here ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

is a warning when you compile in -e prod,
he say envSensorStatus is a unused variable

Copy link
Contributor

Choose a reason for hiding this comment

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

No, we can't merge that and we need to find another solution to fix this. Isn't there a sensor.getStatus() method available on the BME class ?

degrees; there is an offset: 0 maps to -25�C. The default value is
25�C = 0x64, 0x00. As an example 23.5% temperature would be
degrees; there is an offset: 0 maps to -25�C. The default value is
25�C = 0x64, 0x00. As an example 23.5% temperature would be
Copy link
Contributor

Choose a reason for hiding this comment

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

hmmmm those characters are weird ! Are they really responsible for warnings ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not, i don't know from where is coming this.

trap98
trap98 previously requested changes Jul 3, 2018
@@ -86,8 +86,8 @@ void Adafruit_CCS811::setEnvironmentalData(uint8_t humidity, double temperature)
humidity would be 0x61, 0x00.*/

/* Temperature is stored as an unsigned 16 bits integer in 1/512
degrees; there is an offset: 0 maps to -25�C. The default value is
25�C = 0x64, 0x00. As an example 23.5% temperature would be
degrees; there is an offset: 0 maps to -25�C. The default value is
Copy link
Contributor

Choose a reason for hiding this comment

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

special character

@Kameeno Kameeno force-pushed the enhancement/Warning-reduction branch 3 times, most recently from 24fb711 to 0e34425 Compare July 4, 2018 08:04
@@ -658,7 +658,7 @@ void CoolBoard::updateFirmware(String firmwareVersion, String firmwareUrl,
delete this->coolWifi;
INFO_LOG("Starting firmware update...");
t_httpUpdate_return ret =
ESPhttpUpdate.update(firmwareUrl, "", firmwareUrlFingerprint, true);
ESPhttpUpdate.update(firmwareUrl, "", firmwareUrlFingerprint);
Copy link
Contributor

Choose a reason for hiding this comment

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

What does it changes ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

true is a boolean to indicate if need to reboot or not after the ota update, is true for default

// Make sure sensors had enough time to turn on.
// BME280 requires 2ms to start up.
delay(1000);
DEBUG_VAR("BME280 status after begin is:", envSensorStatus);
DEBUG_VAR("BME280 status after begin is:", this->envSensor.begin());
Copy link
Contributor

Choose a reason for hiding this comment

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

No, we can't merge that and we need to find another solution to fix this. Isn't there a sensor.getStatus() method available on the BME class ?

#include "CoolSDS011.h"
#include "SHT1x.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

why change the order ?

@Kameeno Kameeno force-pushed the enhancement/Warning-reduction branch from 0e34425 to 9ae9c11 Compare July 9, 2018 10:39
@Kameeno Kameeno force-pushed the enhancement/Warning-reduction branch from 9ae9c11 to dae7581 Compare July 9, 2018 10:42
@3rwww1 3rwww1 merged commit 2a495cc into master Jul 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants