MS5837 and BMP280 functionality for inches water #21377

vtHydroponics · 2024-05-06T20:54:26Z

Description: Added compatibility with MS5837 and BMP280 to calculate inches of water

Related issue (if applicable): fixes #

Checklist:

[X ] The pull request is done against the latest development branch
[X ] Only relevant files were touched
[X ] Only one feature/fix was added per PR and the code change compiles without warnings
[X ] The code change is tested and works with Tasmota core ESP8266 V.2.7.6
[X ] The code change is tested and works with Tasmota core ESP32 V.3.0.0
[X ] I accept the CLA.

NOTE: The code change must pass CI tests. Your PR cannot be merged unless tests pass

s-hadinger

Thanks. Multiple changes are needed

s-hadinger · 2024-05-06T21:03:05Z

tasmota/include/tasmota_configurations.h

@@ -168,6 +168,7 @@
 //#define USE_LUXV30B // [I2CDriver70] Enable RFRobot SEN0390 LuxV30b ambient light sensor (I2C address 0x4A) (+0k5 code)
 //#define USE_PMSA003I // [I2cDriver78] Enable PMSA003I Air Quality Sensor (I2C address 0x12) (+1k8 code)
 //#define USE_GDK101 // [I2cDriver79] Enable GDK101 sensor (I2C addresses 0x18 - 0x1B) (+1k2 code)
+#define USE_MS5837


Please don't change default configuraiont

s-hadinger · 2024-05-06T21:03:14Z

tasmota/include/tasmota_configurations_ESP32.h

@@ -476,6 +476,7 @@
 //#define USE_LUXV30B // [I2CDriver70] Enable RFRobot SEN0390 LuxV30b ambient light sensor (I2C address 0x4A) (+0k5 code)
 //#define USE_PMSA003I // [I2cDriver78] Enable PMSA003I Air Quality Sensor (I2C address 0x12) (+1k8 code)
 //#define USE_GDK101 // [I2cDriver79] Enable GDK101 sensor (I2C addresses 0x18 - 0x1B) (+1k2 code)
+#define USE_MS5837


s-hadinger · 2024-05-06T21:03:41Z

tasmota/my_user_config.h

@@ -718,6 +718,7 @@
 // #define USE_PCA9557 // [I2cDriver81] Enable PCA9557 8-bit I/O Expander (I2C addresses 0x18 - 0x1F) (+2k5 code)
 // #define USE_MAX17043 // [I2cDriver83] Enable MAX17043 fuel-gauge systems Lipo batteries sensor (I2C address 0x36) (+0k9 code)
 // #define MAX17043_ALERT_THRESHOLD 32 // [I2cDriver83] Define the alert threshold for low battery level percentage 1-32
+ #define USE_MS5837


Please disable by default (just add // before it)

s-hadinger · 2024-05-06T21:04:14Z

tasmota/tasmota_xsns_sensor/xsns_09_bmp.ino

@@ -337,6 +337,8 @@ void Bme280Read(uint8_t bmp_idx) {
 p = ((p + var1 + var2) >> 8) + (((int64_t)Bme280CalibrationData[bmp_idx].dig_P7) << 4);
 bmp_sensors[bmp_idx].bmp_pressure = (float)p / 25600.0f;

+ AddLog(LOG_LEVEL_DEBUG, PSTR("BMP_data{pressure:%f,temperature:%f,bmp_idx:%d}"), bmp_sensors[bmp_idx].bmp_pressure, bmp_sensors[bmp_idx].bmp_temperature, bmp_idx);


Why do you need this? Please remove, the syntax is not consistent with log format in Tasmota

s-hadinger · 2024-05-06T21:05:33Z

tasmota/tasmota_xsns_sensor/xsns_128_ms5837.ino

+ char inchesWater_str[8];
+ dtostrf(inches_water, sizeof(inchesWater_str)-1, 3, inchesWater_str);
+ if (json) {
+ // consolidate to one line


It is easier to use %_f format, see https://tasmota.github.io/docs/Sensor-API/#extension-to-vsnprintf

s-hadinger · 2024-05-06T21:12:12Z

lib/lib_i2c/BlueRobotics_MS5837_Library/MS5837.cpp

+}
+
+float MS5837::altitude() {
+ return (1-pow((pressure()/1013.25),.190284))*145366.45*.3048;


Please use float literals

s-hadinger · 2024-05-06T21:12:52Z

tasmota/tasmota_xsns_sensor/xsns_128_ms5837.ino

+
+#define MS5837_ADDR 0x76
+
+#define XSNS_128 128


Please use 115 instead of 128

s-hadinger · 2024-05-06T21:13:53Z

tasmota/tasmota_xsns_sensor/xsns_128_ms5837.ino

+#define MS5837_ADDR 0x76
+
+#define XSNS_128 128
+#define XI2C_98 98 // See I2CDEVICES.md


Please use next free value, which looks to be 88

s-hadinger · 2024-05-06T21:14:12Z

tasmota/tasmota_xx2c_global/xx2c_interface.ino

@@ -406,7 +406,15 @@ const uint8_t kI2cList[] = {
 #endif

 #ifdef XI2C_96
- XI2C_96


Please revert and use 88

s-hadinger · 2024-05-06T21:15:06Z

tools/decode-status.py

@@ -246,7 +246,7 @@
 "USE_INA219","USE_SHT3X","USE_MHZ19","USE_TSL2561",
 "USE_SENSEAIR","USE_PMS5003","USE_MGS","USE_NOVA_SDS",
 "USE_SGP30","USE_SR04","USE_SDM120","USE_SI1145",
- "USE_SDM630","USE_LM75AD","USE_APDS9960","USE_TM1638"
+ "USE_SDM630","USE_LM75AD","USE_APDS9960","USE_TM1638","USE_MS5837"


I don't see how this is going to work since it was not added to the feature map

arendst · 2024-05-07T07:59:19Z

tasmota/tasmota_xx2c_global/xsns_interface.ino

+#ifdef XSNS_128
+ &Xsns128
+#endif
+


DO NEVER CHANGE ANYTHING IN xsns_interface.ino !!!

This will break sensor enable/disable.

As suggested, use the next free slot.

arendst · 2024-05-07T07:59:40Z

tasmota/tasmota_xx2c_global/xsns_interface.ino

+#ifdef XSNS_128
+ XSNS_128
+#endif
+


arendst · 2024-05-07T08:00:46Z

tasmota/tasmota_xx2c_global/xx2c_interface.ino

+#endif
+
+#ifdef XI2C_98
+ XI2C_98
 #endif


SAME HERE. DO NOT CHANGE xx2c_interface.ino !!!

This breaks I2CDriver command

arendst · 2024-05-07T08:03:55Z

The goal of a PR is to add functionality with touching as few files as possible.

You're touching files with adding spaces and irrelevant logs. There is even a TSL file change not related to your initial addition.

Pls revisit your PR and make as few changes as possible using the above suggestions.

vtHydroponics added 7 commits November 12, 2023 21:02

Changes from 11/12/2023. Switches made, need debug

6e49e16

Finalized gain/integration adjustment trees

738bd23

Fixed the bugs

1d3d55c

Outputs in JSON, hex displays as hex was decimal

9def89b

works but polishing code

04335ba

need to debug pressure in bmp

ee0fb81

updated temp to change via setoption8 command from tasmota

8ec2629

s-hadinger requested changes May 6, 2024

View reviewed changes

arendst reviewed May 7, 2024

View reviewed changes

tasmota/tasmota_xx2c_global/xsns_interface.ino

#ifdef XSNS_128

XSNS_128

#endif

Copy link

Owner

arendst May 7, 2024

Choose a reason for hiding this comment

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

Ditto !!!

arendst reviewed May 7, 2024

View reviewed changes

arendst added the on hold by dev team Result - Feature request put on hold by member of development team label May 7, 2024

Jason2866 marked this pull request as draft May 7, 2024 15:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MS5837 and BMP280 functionality for inches water #21377

MS5837 and BMP280 functionality for inches water #21377

vtHydroponics commented May 6, 2024

s-hadinger left a comment

s-hadinger May 6, 2024

s-hadinger May 6, 2024

s-hadinger May 6, 2024

s-hadinger May 6, 2024

s-hadinger May 6, 2024

s-hadinger May 6, 2024

s-hadinger May 6, 2024

s-hadinger May 6, 2024

s-hadinger May 6, 2024

s-hadinger May 6, 2024

arendst May 7, 2024

arendst May 7, 2024

arendst May 7, 2024

arendst commented May 7, 2024

MS5837 and BMP280 functionality for inches water #21377

Are you sure you want to change the base?

MS5837 and BMP280 functionality for inches water #21377

Conversation

vtHydroponics commented May 6, 2024

Description: Added compatibility with MS5837 and BMP280 to calculate inches of water

Checklist:

s-hadinger left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arendst commented May 7, 2024