Skip to content

Fix imap_email_content unknown status and replaying stale states#89563

Merged
bdraco merged 4 commits into
home-assistant:devfrom
jbouwh:fix-imap_content-unknown-status
Mar 20, 2023
Merged

Fix imap_email_content unknown status and replaying stale states#89563
bdraco merged 4 commits into
home-assistant:devfrom
jbouwh:fix-imap_content-unknown-status

Conversation

@jbouwh
Copy link
Copy Markdown
Contributor

@jbouwh jbouwh commented Mar 12, 2023

Breaking change

Only for the last valid mail the state is updated at startup.
During startup emails received since yesterday are examined, this was since today.
The state will not become unknown if no new messages are received after a previous valid state, unless home_assistant is restarted.

Proposed change

This PR changes the startup behaviour for the imap_email_content sensor platform.

  • Emails are examined for today and yesterday, this avoids unknown states when the date is changing and there is not an update email to process yet.
  • Only for the last valid received mail the sensor state is updated.

Note

The imap_email_content sensor misses an option that allows to set wat retain period the last state message should have. Now this is min 1 day, and max 2 days.
To allow further improvements this integration should be updated to be set up using config flow. A PR was opened for this : #89707.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Deprecation (breaking change to happen in the future)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • I have followed the perfect PR recommendations
  • The code has been formatted using Black (black --fast homeassistant tests)
  • Tests have been added to verify that the new code works.

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

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
  • Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

@frenck frenck added the smash Indicator this PR is close to finish for merging or closing label Mar 14, 2023
Comment thread homeassistant/components/imap_email_content/sensor.py Outdated
Comment on lines +279 to +293

if self.sender_allowed(email_message):
message = EmailContentSensor.get_msg_subject(email_message)

if self._value_template is not None:
message = self.render_template(email_message)

self._message = message
self._state_attributes = {
ATTR_FROM: EmailContentSensor.get_msg_sender(email_message),
ATTR_SUBJECT: EmailContentSensor.get_msg_subject(email_message),
ATTR_DATE: email_message["Date"],
ATTR_BODY: EmailContentSensor.get_msg_text(email_message),
}

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.

It seems like this should only happen if we are going to break out of the loop instead of processing it and than throwing it away when we hit the next one?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We do not know if an unseen mail has an allowed sender unless we process it. The current code base uses a custom EmailReader class as backend.
The code in dev needs several update iterations before it reaches the last one.
The improvement here is to iterate the messages until we reach the last one in a single update cycle.

@bdraco bdraco self-requested a review March 17, 2023 23:37
ATTR_BODY: EmailContentSensor.get_msg_text(email_message),
}

if self._last_id == self._email_reader.last_unread_id:
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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'll have a look at that later today.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

self._email_reader.last_id is the last message processed, self._email_reader.last_unread_id is the last message to process. The condition is false when we are at startup and we searched all messages for the last 2 days. There might be more messages that match the IMAP search, but we do not known if the sender matches. When the last message is processed the last state set will be come the new state. If the last message processed is not the last message to process, then we read a new message until we reach the last one. To simulate this, place to matching new message in the mailbox and startup home assistant.

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.

Makes sense now that I'm back at my desktop. Apparently I just needed a slightly larger monitor to see that 😉

@bdraco
Copy link
Copy Markdown
Member

bdraco commented Mar 19, 2023

Screenshot 2023-03-18 at 2 14 54 PM

Working as expected. Please see #89563 (comment)

@bdraco bdraco merged commit 2039955 into home-assistant:dev Mar 20, 2023
@bdraco
Copy link
Copy Markdown
Member

bdraco commented Mar 20, 2023

Thanks @jbouwh

@jbouwh jbouwh deleted the fix-imap_content-unknown-status branch March 20, 2023 05:59
@jbouwh jbouwh added this to the 2023.3.6 milestone Mar 20, 2023
@github-actions github-actions Bot locked and limited conversation to collaborators Mar 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Issue with IMAP content integration

4 participants