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

Increase internal time #1163

Merged
merged 4 commits into from
Jan 11, 2024
Merged

Increase internal time #1163

merged 4 commits into from
Jan 11, 2024

Conversation

arne182
Copy link
Contributor

@arne182 arne182 commented Dec 13, 2023

Every second is a bit overkill causing home assistant to struggle to visualise the data. Every 5 seconds should be better.

Every second is a bit overkill causing home assistant to struggle to visualise the data. Every 5 seconds should be better.
Copy link
Contributor

@DTTerastar DTTerastar left a comment

Choose a reason for hiding this comment

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

There was a request to do the opposite. So I think we need configuration.

@arne182
Copy link
Contributor Author

arne182 commented Dec 14, 2023

I agree. An option to specify how many times the reading is done would be useful.

@arne182 arne182 closed this Dec 14, 2023
@arne182
Copy link
Contributor Author

arne182 commented Dec 15, 2023

@DTTerastar could you at least make a beta release with one reading every minute until you have the time to make it into a parameter?

@arne182
Copy link
Contributor Author

arne182 commented Dec 16, 2023

At least for this sensor

@arne182 arne182 reopened this Dec 16, 2023
@FvanEgmond
Copy link
Contributor

Good day, I do agree with @arne182 that 60 seconds is at least a better default interval -at least for now until a user-customizable interval is programmed maybe later- as CO2 and TVOC levels do not change radically within minutes. I therefore sincerely hope that this pull request will be approved soon.
@DTTerastar, can you set expectations what the chances are that this can be pulled into a new release, and when that is (loosely) planned? :-) Thanks in advance for your reaction. Cheers, Frank

@DTTerastar DTTerastar enabled auto-merge (squash) January 11, 2024 02:12
@DTTerastar DTTerastar merged commit d888c45 into ESPresense:master Jan 11, 2024
4 of 16 checks passed
@arne182
Copy link
Contributor Author

arne182 commented Jan 11, 2024

Thanks.

@AKHwyJunkie
Copy link

I was looking into this as well. You might be interested to know that the SGP30 (and 40/41) were designed to operate at 1Hz, or once every second. So, a change would actually go against the chip manufacturer's recommended polling value. I think the ideal resolution for everyone would be to parameterize all sensor reporting time, per sensor. Then, users could choose if they want to run in spec or out of spec. This is obviously much larger in scope, though.

@arne182
Copy link
Contributor Author

arne182 commented Jan 16, 2024

I don't mind the data to be polled every second but the Mqtt reported is a problem for gone assistant to handle. Especially multiple sensors. It would in ideal to have it poll every second and report if a certain percentage change is reached or the minimum report time is reached. Allowing for the reporting interval to be variable.

@FvanEgmond
Copy link
Contributor

FvanEgmond commented Jan 17, 2024

Thank you @AKHwyJunkie for this insights. I have the feeling, testing the latest beta with pulling interval of 60 seconds, that the values reported are indeed behaving slightly different than when the pulling interval was 1 second.
I do agree with @arne182 that the best solution is to split the loop in two: pull every second (with SensorInterval = 1) and report every 60 seconds (with a new variable ReportInterval).
In later releases, this can be extended to have a flexible report interval, and taking the max or average value of all the values pulled in the seconds in between. How does that sound? :-)

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.

None yet

4 participants