Skip to content

Change STATE_UNKOWN to None#20337

Merged
MartinHjelmare merged 12 commits intodevfrom
state_unkown
Jan 24, 2019
Merged

Change STATE_UNKOWN to None#20337
MartinHjelmare merged 12 commits intodevfrom
state_unkown

Conversation

@Danielhiversen
Copy link
Copy Markdown
Member

Description:

Change STATE_UNKOWN to None

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.

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

  • New dependencies have been added to the REQUIREMENTS variable ([example][ex-requir]).
  • New dependencies are only imported inside functions that use them ([example][ex-import]).
  • New or updated dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
  • New files were added to .coveragerc.

@Danielhiversen Danielhiversen requested review from a team, cdce8p and fabaff as code owners January 23, 2019 06:56
@ghost ghost assigned Danielhiversen Jan 23, 2019
@ghost ghost added the in progress label Jan 23, 2019
if value == comp_value:
return key
return STATE_UNKNOWN
return None
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.

The changed function here creates state attributes, so this will be a breaking change. State attributes are not parsed, the way state is parsed, converting None to STATE_UNKNOWN in the base entity class.

for state in self._data.status[KEY_STATUS].split())
except KeyError:
return STATE_UNKNOWN
return None
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.

This function also populates state attributes, so will be a breaking change.

"""Update state data from Xbox API."""
presence = self._api.get_user_presence(self._xuid)
self._state = presence.get('state', STATE_UNKNOWN)
self._state = presence.get('state', None)
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.

If it's a dictionary the default returned value is None when the key is missing.

if match:
return match.group(1)
return STATE_UNKNOWN
return None
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.

This method is also used to populate state attributes, so this will be a breaking change.

if standby_state == WEMO_STANDBY:
return STATE_STANDBY
return STATE_UNKNOWN
return None
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.

Same as above.

Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

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

Looks good!

@ghost ghost assigned balloob Jan 24, 2019
@MartinHjelmare MartinHjelmare merged commit 1bd31e3 into dev Jan 24, 2019
@ghost ghost removed the in progress label Jan 24, 2019
@delete-merged-branch delete-merged-branch bot deleted the state_unkown branch January 24, 2019 07:20
@balloob balloob mentioned this pull request Feb 6, 2019
alandtse pushed a commit to alandtse/home-assistant that referenced this pull request Feb 12, 2019
* Change STATE_UNKOWN to None

* Change STATE_UNKOWN to None

* tests

* tests

* tests

* tests

* tests

* style

* fix comments

* fix comments

* update fan test
kellerza pushed a commit to kellerza/ha-core that referenced this pull request Feb 24, 2019
* Change STATE_UNKOWN to None

* Change STATE_UNKOWN to None

* tests

* tests

* tests

* tests

* tests

* style

* fix comments

* fix comments

* update fan test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants