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.

Sign up for GitHub

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

Jump to bottom

Change to use getlogger #36

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Change to use getlogger #36

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
f071550
Create hacs.json
johro897 Sep 14, 2023
8cb6a49
First Logging change
johro897 Sep 14, 2023
887492e
Updated
johro897 Sep 17, 2023
fd6dd64
update
johro897 Sep 17, 2023
a515292
update
johro897 Sep 18, 2023
c3ef09d
Update of all loggers
johro897 Sep 18, 2023
4707a0f
removed old comments
johro897 Sep 21, 2023
9ff02f9
Merge pull request #1 from johro897/fix_logging
johro897 Sep 21, 2023
b43af58
Update .gitignore
johro897 Sep 21, 2023
95a2a6f
Update .gitignore
johro897 Sep 21, 2023
7f1a4da
Delete hacs.json
johro897 Sep 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

Copy link
Owner

Choose a reason for hiding this comment

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

Mind leaving this change out as it's not related to the _LOGGER change?

.DS_Store
*.pyc
.vscode/*
8 changes: 5 additions & 3 deletions custom_components/oura/helpers/hass_helper.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,15 @@

import logging

_LOGGER = logging.getLogger(__name__)

# Safe importing as this module was not existing on previous versions
# of Home-Assistant.
try:
from homeassistant.helpers import network
except ModuleNotFoundError:
logging.debug('Network module not found.')
_LOGGER.debug('Network module not found.')



def get_url(hass):
Expand All @@ -29,8 +32,7 @@ def get_url(hass):
prefer_external=True,
require_ssl=False)
except AttributeError:
logging.debug(
'Hass version does not have get_url helper, using fall back.')
_LOGGER.debug('Hass version does not have get_url helper, using fall back.')
Copy link
Owner

Choose a reason for hiding this comment

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

This line goes over 80 chars. Do you mind breaking in two similar to how the original was?


base_url = hass.config.api.base_url
if base_url:
Expand Down
1 change: 1 addition & 0 deletions custom_components/oura/sensor_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from homeassistant.helpers import entity
from . import api

_LOGGER = logging.getLogger(__name__)
Copy link
Owner

Choose a reason for hiding this comment

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

Actually, _LOGGER is not used in this file. I think a better change would be removing line 9 and import logging on line 3 altogether. My bad.

SENSOR_NAME = 'oura'


Expand Down
24 changes: 10 additions & 14 deletions custom_components/oura/sensor_base_dated.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from . import sensor_base
from .helpers import date_helper

_LOGGER = logging.getLogger(__name__)

class MonitoredDayType(enum.Enum):
"""Types of days which can be monitored."""
Expand Down Expand Up @@ -152,9 +153,7 @@ def _get_date_by_name(self, date_name):
days_ago = None

if days_ago is None:
logging.info(
f'Oura ({self._name}): ' +
'Unknown day name `{date_name}`, using yesterday.')
_LOGGER.info('Unknown day name ' + date_name + 'using yesterday.')
Copy link
Owner

Choose a reason for hiding this comment

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

Change to f-strings and keep self._name. The first change would ensure higher consistency and readability. The second would help in troubleshooting when multiple Oura sensors are added.

days_ago = 1

return str(today - datetime.timedelta(days=days_ago))
Expand Down Expand Up @@ -253,15 +252,13 @@ def _map_data_to_monitored_days(self, sensor_data, default_attributes=None):
backfill += 1

if original_date != date_value:
logging.warning(
(
f'Oura ({self._name}): No Oura data found for '
f'{date_name_title} ({original_date}). Fetching {date_value} '
'instead.'
) if date_value else (
f'Unable to find suitable backfill date. No data available.'
)
)
if date_value:
message = 'No Oura data found for '+ date_name_title +' ('+ original_date +'). Fetching '+ date_value + 'instead.'
Copy link
Owner

Choose a reason for hiding this comment

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

This has an issue because there's no space before instead, so the date would merge. Also space before and after spaces are desirable for readability.

To solve for all this, let's change to f-strings here and 258.

Copy link
Owner

Choose a reason for hiding this comment

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

This comment is missing self._name which might be helpful for debugging.

Copy link
Owner

Choose a reason for hiding this comment

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

This exceeds 80 chars. Break it into lines of max 80 chars. That's why the original is broken down into concatenated strings.

else:
message = 'Unable to find suitable backfill date. No data available.'

_LOGGER.warning(message)


if daily_data:
date_attributes.update(daily_data)
Expand Down Expand Up @@ -362,8 +359,7 @@ def parse_sensor_data(self, oura_data, data_param='data', day_param='day'):
Oura sensor data for that given day.
"""
if not oura_data or data_param not in oura_data:
logging.error(
f'Oura ({self._name}): Couldn\'t fetch data for Oura ring sensor.')
_LOGGER.error('Couldnt fetch data for Oura ring sensor.')
Copy link
Owner

Choose a reason for hiding this comment

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

Couldn't - to avoid probelsm with the single quotes, you can escape it 'Couldn't ...'

Aso, this is missing self._name. It might be worth just leaving Oura ({self._name}) on all of them even if this is now done by _LOGGER which will include the module name.

return {}

sensor_data = oura_data.get(data_param)
Expand Down
18 changes: 7 additions & 11 deletions custom_components/oura/sensor_base_dated_series.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import logging
from . import sensor_base_dated

_LOGGER = logging.getLogger(__name__)

class OuraDatedSeriesSensor(sensor_base_dated.OuraDatedSensor):
"""Representation of an Oura Ring sensor with series daily data.
Expand Down Expand Up @@ -91,15 +92,11 @@ def _map_data_to_monitored_days(self, sensor_data, default_attributes=None):
backfill += 1

if original_date != date_value:
logging.warning(
(
f'Oura ({self._name}): No Oura data found for '
f'{date_name_title} ({original_date}). Fetching {date_value} '
'instead.'
) if date_value else (
f'Unable to find suitable backfill date. No data available.'
)
)
if date_value:
Copy link
Owner

Choose a reason for hiding this comment

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

Can we keep the original if logic structure?

message = 'No Oura data found for '+ date_name_title +' ('+ original_date +'). Fetching '+ date_value + 'instead.'
Copy link
Owner

Choose a reason for hiding this comment

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

Ditto: change to f-strings, keep lines below 80 chars and keep Oura ({self._name})

else:
message = 'Unable to find suitable backfill date. No data available.'
_LOGGER.warning(message)
Copy link
Owner

Choose a reason for hiding this comment

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

nit: to keep consistent with the other file (e.g.sensor_base_dated lines 258-260) add a break line before _LOGGER.warning()


Copy link
Owner

Choose a reason for hiding this comment

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

Ditto from before: change to f-strings and keep Oura ({self._name}.

if not daily_data:
daily_data = [self._empty_sensor]
Expand Down Expand Up @@ -166,8 +163,7 @@ def parse_sensor_data(self, oura_data, data_param='data', day_param='day'):
Oura sensor data for that given day.
"""
if not oura_data or data_param not in oura_data:
logging.error(
f'Oura ({self._name}): Couldn\'t fetch data for Oura ring sensor.')
_LOGGER.error('Couldnt fetch data for Oura ring sensor.')
Copy link
Owner

Choose a reason for hiding this comment

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

Ditto: Couldn\'t and keep Oura ({self._name})

return {}

sensor_data = oura_data.get(data_param)
Expand Down