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 room name information to Position object #397

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from

Conversation

francescopalma86
Copy link

No description provided.

edenhaus and others added 3 commits January 18, 2024 19:39
added room_id to Position obj

current_room_id on device

fix

add room_id on PositionEvent

removed current_room_id

semplified get and set device rooms

added room_name to position

fix map property don't used yet

expose room name in position object
Copy link
Contributor

@edenhaus edenhaus left a comment

Choose a reason for hiding this comment

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

Please make sure the CI is passing

Comment on lines +102 to +103
if self.name == "getPos":
response["resp"]["body"]["data"]["rooms"] = rooms
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this needed?
getPos specific code should go into the GetPos class and not in the general Command class.

Copy link
Author

@francescopalma86 francescopalma86 Jan 22, 2024

Choose a reason for hiding this comment

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

I don't know how to pass rooms parameter to GetPos class. This is the only way I've found. Help me?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this code should be in map.py and not in the command. The commands module is what we get from the API and we should not do calculations there.
Instead I think the calculation should be part of map.py, where you already have the rooms and the position

@@ -80,6 +85,12 @@ async def on_pos(event: PositionsEvent) -> None:

self.events.subscribe(PositionsEvent, on_pos)

async def on_rooms(event: RoomsEvent) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should go into map.py

@@ -407,7 +408,10 @@ async def on_map_set(event: MapSetEvent) -> None:

async def on_map_subset(event: MapSubsetEvent) -> None:
if event.type == MapSetType.ROOMS and event.name:
room = Room(event.name, event.id, event.coordinates)
decompressed_coords = _decompress_7z_base64_data(event.coordinates).decode('utf-8')
Copy link
Contributor

Choose a reason for hiding this comment

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

Compressed coordinates are Part of GetMapSubset_V2 but not the GetMapSubset and therefore should be in the command class otherwise this code will break older bots

requirements.txt Outdated Show resolved Hide resolved
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.

2 participants