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

Fix mood handling #260

Merged
merged 9 commits into from
Jan 4, 2020
Merged

Fix mood handling #260

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
7013a72
fix mood handling (moods are always associated with groups)
Rob4t Dec 26, 2019
c640bf7
fix getting active mood for group
Rob4t Dec 26, 2019
5bc78a0
make available moods part of a group
Rob4t Dec 26, 2019
bf28c0e
remove block comment
Rob4t Dec 26, 2019
8badc91
add tests for moods
rgci Jan 2, 2020
29945ff
fix missing whitespace and blank lines
rgci Jan 2, 2020
0dfe2d6
another missing blank line in test_group
rgci Jan 2, 2020
d7e9d3c
fix another missing blank line and whitespace
rgci Jan 2, 2020
f82c1e2
re-add moods to main
Rob4t Jan 4, 2020
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
7 changes: 0 additions & 7 deletions pytradfri/__main__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,6 @@
else:
print("No groups found!")
group = None
moods = api(gateway.get_moods())
Copy link
Member

Choose a reason for hiding this comment

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

I'd like to keep this in main. Can we add try/catch instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you expect a flattened list of moods or a dict like:
{
"Group1": [ ...]
}
The former would not be very useful without the information for which group the mood is available (since there are no "global" moods)

Copy link
Member

Choose a reason for hiding this comment

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

I wonder if that's why moods are no longer working, they might have been global when we wrote the code. The purpose of pytradfri is to reflect what data is provided by the gateway without any manipulation. Is the group included in the raw data passed by the gateway?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately it is not ( see tests/moods.py ). Also the raw data of a group only has its current active mood_id.
So we have to use group.moods() to fetch the available moods for the given group

Copy link
Member

Choose a reason for hiding this comment

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

We should only return what the gateway returns, no logic nor manipulation of the data. That has to be done by the consumer in its implementation of pytradfri. That leaves us at option #1 I guess?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Also added the parent_id (group id) to the mood's repr

if moods:
mood = moods[0]
else:
print("No moods found!")
mood = None
tasks = api(gateway.get_smart_tasks())
homekit_id = api(gateway.get_gateway_info()).homekit_id

Expand Down Expand Up @@ -112,7 +106,6 @@ def dump_devices():
print("> tasks[0].repeat_days_list")
print("> api(gateway.reboot())")
print("> groups")
print("> moods")
print("> tasks")
print("> dump_devices()")
print("> dump_all()")
24 changes: 6 additions & 18 deletions pytradfri/gateway.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,19 +103,20 @@ def process_result(result):
[ROOT_GATEWAY, ATTR_GATEWAY_INFO],
process_result=process_result)

def get_moods(self):
def get_moods(self, group_id):
"""
Return moods defined on the gateway.
Return moods available in given group.

Returns a Command.
"""
mood_parent = self._get_mood_parent()
#mood_parent = self._get_mood_parent()
Copy link

Choose a reason for hiding this comment

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

block comment should start with '# '

#TODO check groupid
Copy link

Choose a reason for hiding this comment

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

block comment should start with '# '


def process_result(result):
return [self.get_mood(mood, mood_parent=mood_parent) for mood in
return [self.get_mood(mood, mood_parent=group_id) for mood in
result]

return Command('get', [ROOT_MOODS, mood_parent],
return Command('get', [ROOT_MOODS, group_id],
process_result=process_result)

def get_mood(self, mood_id, *, mood_parent=None):
Expand All @@ -124,26 +125,13 @@ def get_mood(self, mood_id, *, mood_parent=None):

Returns a Command.
"""
if mood_parent is None:
mood_parent = self._get_mood_parent()

def process_result(result):
return Mood(result, mood_parent)

return Command('get', [ROOT_MOODS, mood_parent, mood_id],
mood_parent, process_result=process_result)

def _get_mood_parent(self):
"""
Get the parent of all moods.

Returns a Command.
"""
def process_result(result):
return result[0]

return Command('get', [ROOT_MOODS], process_result=process_result)

def get_smart_tasks(self):
"""
Return the transitions linked to the gateway.
Expand Down
9 changes: 7 additions & 2 deletions pytradfri/group.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,19 @@ def members(self):
"""Return device objects of members of this group."""
return [self._gateway.get_device(dev) for dev in self.member_ids]

def moods(self):
"""Return mood objects of moods in this group."""
return self._gateway.get_moods(self.id)

def mood(self):
""""Active mood."""
return self._gateway.get_mood(self.mood_id)
return self._gateway.get_mood(self.mood_id, mood_parent=self.id)

def activate_mood(self, mood_id):
"""Activate a mood."""
return self.set_values({
ATTR_MOOD: mood_id
ATTR_MOOD: mood_id,
ATTR_DEVICE_STATE: int(self.state)
})

def set_state(self, state):
Expand Down