Skip to content

Conversation

@woczach
Copy link

@woczach woczach commented Mar 19, 2025

Description:

Add sensor:
https://wiki.dfrobot.com/_SKU_SEN0377_Gravity__MEMS_Gas_Sensor_CO__Alcohol__NO2___NH3___I2C___MiCS_4514#target_7

Related issue (if applicable): fixes #

Checklist:

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

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

@Jason2866
Copy link
Collaborator

revert changes in files not related to the driver. Take a look in other drivers how Tasmota logging is done

@Jason2866 Jason2866 requested a review from Copilot March 20, 2025 16:15
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request adds support for the DFRobot MICS-4514 sensor by including a new README that documents the sensor's features, installation steps, and API methods.

  • Adds the README documentation for the sensor library.
  • Provides detailed method descriptions and usage examples for sensor functionalities.
Files not reviewed (12)
  • .vscode/settings.json: Language not supported
  • lib/lib_i2c/DFRobot_MICS-master/DFRobot_MICS.h: Language not supported
  • lib/lib_i2c/DFRobot_MICS-master/LICENSE: Language not supported
  • lib/lib_i2c/DFRobot_MICS-master/examples/enablePower/enablePower.ino: Language not supported
  • lib/lib_i2c/DFRobot_MICS-master/examples/getADCData/getADCData.ino: Language not supported
  • lib/lib_i2c/DFRobot_MICS-master/examples/getGasExist/getGasExist.ino: Language not supported
  • lib/lib_i2c/DFRobot_MICS-master/examples/getGasPPM/getGasPPM.ino: Language not supported
  • lib/lib_i2c/DFRobot_MICS-master/keywords.txt: Language not supported
  • lib/lib_i2c/DFRobot_MICS-master/library.properties: Language not supported
  • tasmota/my_user_config.h: Language not supported
  • tasmota/tasmota_xsns_sensor/xsns_120_dfrobot_mics.ino: Language not supported
  • tasmota/tasmota_xx2c_global/xx2c_interface.ino: Language not supported
Comments suppressed due to low confidence (1)

lib/lib_i2c/DFRobot_MICS-master/README.md:34

  • [nitpick] Consider using a consistent dash character (e.g., '–') instead of '-' to maintain uniform formatting in the measurement range descriptions.
1000 - ∞ ppm(Methane CH4 )

@arendst arendst added the awaiting feedback Action - Waiting for response or more information label Mar 22, 2025
@woczach
Copy link
Author

woczach commented Apr 7, 2025

Hi @Jason2866 I reverted changes in unrelated files, and fixed logging. Best

@woczach
Copy link
Author

woczach commented Apr 10, 2025

revert changes in files not related to the driver. Take a look in other drivers how Tasmota logging is done

@woczach woczach closed this Apr 10, 2025
@woczach woczach reopened this Apr 10, 2025
switch (DFRobot_MICS_data.loop_count) {
case 15:
tempFloat = mics.getGasData(CH4);
if (tempFloat > DFRobot_MICS_data.Methane) DFRobot_MICS_data.Methane = tempFloat;
Copy link
Owner

Choose a reason for hiding this comment

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

Do you expect only to read higher new values? Comparing new value to old value this way will never show lower values....

Copy link
Author

Choose a reason for hiding this comment

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

It was obvious for gas existence, and I am bit not sure, as I consider gas detector, as kind of canary and we sent data like every 5min i want highest value. I don't want last zero value to hide positive high value that was before.

Copy link
Contributor

Choose a reason for hiding this comment

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

IMO, reporting false values to satisfy a specific intention should not be how a sensor driver works, but instead to be handled backend where you can react to the changing values, and decide what to do after getting an alarmingly high value.

Connecting to an ESP32, you could also use Berry to create logic about how to react to changing values, and even create an additional sensor keeping up the max even after the real reading fell again.

char Nitric_Oxide_b[8];
char Nitrogen_Dioxide_b[8];

} DFRobot_MICS_ch;
Copy link
Owner

Choose a reason for hiding this comment

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

This is very expensive memory use.

Redesign your code to use floats and ints from DFRobot_MICS_data. Then use Tasmota specific "float to string" code like

ResponseAppend_P(PSTR(",\"DFRobot_MICS\":{"\"Methane\":\"%2_f\"...,&DFRobot_MICS_data.Methane,...

Copy link
Author

Choose a reason for hiding this comment

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

ofc will do

@github-actions
Copy link

github-actions bot commented May 6, 2025

This PR has been automatically marked as stale because it hasn't any activity in last few weeks. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale Action - Issue left behind - Used by the BOT to call for attention label May 6, 2025
@github-actions
Copy link

This PR was automatically closed because of being stale.

@github-actions github-actions bot closed this May 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting feedback Action - Waiting for response or more information stale Action - Issue left behind - Used by the BOT to call for attention

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants