8000
Skip to content

Fix ezo parsing - #4792

Merged
jesserockz merged 4 commits into
esphome:devfrom
maisken:fix-ezo-parsing
May 9, 2023
Merged

Fix ezo parsing#4792
jesserockz merged 4 commits into
esphome:devfrom
maisken:fix-ezo-parsing

Conversation

@alfredopironti
@alfredopironti alfredopironti commented May 8, 2023
Copy link
Copy Markdown
Contributor

What does this implement/fix?

The current parsing of the EZO sensor had a few issues that wouldn't make them work. Namely:

  • The returned value would be truncated always (not only for EZO_READ), which would not allow any other command to actually work (cd5dae3)
  • The temperature-compensated readings were not properly parsed (f91f691)
  • Additionally, the component was previously logging binary data (f2dfba2)

This is tested against both a pH and ORP EZO sensors. I've carefully reviewed all EZO datasheets, and they all follow the same pattern, so this code should work with every EZO sensor available at time of writing. The only caveat is the RGB EZO sensor which, by nature, on EZO_READ returns 3 comma-separated values (R,G,B). The current implementation only returns the "R" component. Support for the RGB sensor entails a breaking change, where "EZO_READ" returns an array of values. I'd be happy to implement such a change, but didn't want to introduce a breaking change at this stage.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Other

Related issue or feature (if applicable): N/A

Pull request in esphome-docs with documentation (if applicable): N/A (no documentation updates required)

Test Environment

  • ESP32
  • ESP32 IDF
  • ESP8266
  • RP2040

Example entry for config.yaml:

Typical use case reading the EZO_CALIBRATION status:

button:
  - platform: template
    name: "Retrieve pH calibration status"
    on_press:
      - lambda: |-
         id(ezo_ph).get_calibration();

sensor:
  - platform: ezo
    id: ezo_ph
    address: 99
    name: "pH sensor"
    unit_of_measurement: "pH"
    accuracy_decimals: 2
    on_calibration:
      - text_sensor.template.publish:
          id: ph_calibration_status
          state: !lambda |-
            return x;

Without this patch, the "on_calibration" callback is never invoked, due to parsing issues.

Checklist:

  • The code change is tested and works locally.
  • Tests have been added to verify that the new code works (under tests/ folder).

If user exposed functionality or configuration variables are added/changed:

@alfredopironti
Don't log the first byte, since it's binary
f2dfba2
@alfredopironti
Fix message parsing #1
cd5dae3 
- Only proceed if first byte is 1
(previous code would proceed nevertheless
in case of EZO_CALIBRATION commands)

- Truncate returned value only for EZO_READ commands
(previous code would truncate it always, making parsing impossible)
@alfredopironti
Fix message parsing #2
f91f691 
Temperature-compensated readings (EZO_T) behave like regular commands,
so they need the same parsing.
@alfredopironti
alfredopironti requested a review from ssieb as a code owner May 8, 2023 13:02
@probot-esphome
probot-esphome Bot commented May 8, 2023
Copy link
Copy Markdown

Hey there @ssieb, mind taking a look at this pull request as it has been labeled with an integration (ezo) you are listed as a code owner for? Thanks!
(message by CodeOwnersMention)

jesserockz
jesserockz approved these changes May 9, 2023
View reviewed changes
@jesserockz jesserockz left a comment
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can't test it, but code looks fine.

@jesserockz
jesserockz merged commit b5dac00 into esphome:dev May 9, 2023
@jesserockz jesserockz mentioned this pull request May 10, 2023
Merged
@github-actions github-actions Bot locked and limited conversation to collaborators May 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jesserockz jesserockz jesserockz approved these changes
@ssieb ssieb Awaiting requested review from ssieb ssieb is a code owner

Assignees

@ssieb ssieb

Labels

component: ezo small-pr PR < 30 lines

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@alfredopironti @jesserockz @Assunto @TheRealFalseReality @ssieb
0