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

Added time left for recommended sensor cleaning #119

Merged
merged 5 commits into from
Nov 17, 2017

Conversation

bbbenji
Copy link
Contributor

@bbbenji bbbenji commented Nov 16, 2017

Unfortunately I am unable to test these changes, however I believe they should work.

Unfortunately I am unable to test these changes, however I believe they should work.
@property
def sensor_dirty_left(self) -> timedelta:
return self.side_brush_total - self.sensor_dirty

Choose a reason for hiding this comment

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

blank line contains whitespace

@@ -292,6 +292,7 @@ def __init__(self, data: Dict[str, Any]) -> None:
self.main_brush_total = timedelta(hours=300)
self.side_brush_total = timedelta(hours=200)
self.filter_total = timedelta(hours=150)
self.sensors_total = timedelta(hours=30)

Choose a reason for hiding this comment

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

trailing whitespace

@coveralls
Copy link

coveralls commented Nov 16, 2017

Coverage Status

Coverage increased (+0.008%) to 45.008% when pulling 45281bc on bbbenji:patch-1 into ed01fd8 on rytilahti:master.

Github browser based editor sucks!
@coveralls
Copy link

coveralls commented Nov 16, 2017

Coverage Status

Coverage increased (+0.008%) to 45.008% when pulling f3f8fbe on bbbenji:patch-1 into ed01fd8 on rytilahti:master.

Copy link
Owner

@rytilahti rytilahti left a comment

Choose a reason for hiding this comment

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

Thanks for your patch! I added a couple of comments that needs to be handled before merging this. Could you also please adjust vacuum_cli.py to print out the left value as it is done for other consumables?

@@ -292,6 +292,7 @@ def __init__(self, data: Dict[str, Any]) -> None:
self.main_brush_total = timedelta(hours=300)
self.side_brush_total = timedelta(hours=200)
self.filter_total = timedelta(hours=150)
self.sensors_total = timedelta(hours=30)
Copy link
Owner

Choose a reason for hiding this comment

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

Use singular here, as property is "sensor_dirty". Or if you thing it's worth renaming it now, maybe introduce a new, renamed property and @deprecate the old one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for taking the time to have a look. I have fixed the issues.

@@ -321,6 +322,10 @@ def filter_left(self) -> timedelta:
def sensor_dirty(self) -> timedelta:
return pretty_seconds(self.data["sensor_dirty_time"])

@property
def sensor_dirty_left(self) -> timedelta:
return self.side_brush_total - self.sensor_dirty
Copy link
Owner

Choose a reason for hiding this comment

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

side_brush_total is wrong.

@coveralls
Copy link

coveralls commented Nov 17, 2017

Coverage Status

Coverage increased (+0.008%) to 45.008% when pulling 1e048a2 on bbbenji:patch-1 into ed01fd8 on rytilahti:master.

@@ -324,7 +324,7 @@ def sensor_dirty(self) -> timedelta:

@property
def sensor_dirty_left(self) -> timedelta:
return self.side_brush_total - self.sensor_dirty
return self.sensor_dirty_total - self.sensor_dirty
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 still wrong :-) Also, please update vacuum_cli.py accordingly to display the new property.

@coveralls
Copy link

coveralls commented Nov 17, 2017

Coverage Status

Coverage increased (+0.008%) to 45.008% when pulling c50a454 on bbbenji:patch-1 into ed01fd8 on rytilahti:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+0.008%) to 45.008% when pulling c50a454 on bbbenji:patch-1 into ed01fd8 on rytilahti:master.

@coveralls
Copy link

coveralls commented Nov 17, 2017

Coverage Status

Coverage increased (+0.008%) to 45.008% when pulling 0466dd8 on bbbenji:patch-1 into ed01fd8 on rytilahti:master.

@bbbenji
Copy link
Contributor Author

bbbenji commented Nov 17, 2017

I am far from a programmer. Hope this is everything done correctly:) Thanks for your patience.

@rytilahti
Copy link
Owner

Seems to work fine, I just tested it locally :-)

Sensor dirty: 1 day, 14:42:18 (left -1 day, 15:17:42)

(cleaning the sensors / reseting the status is overdue, it seems..)

@rytilahti rytilahti merged commit bd5e346 into rytilahti:master Nov 17, 2017
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.

4 participants