PiotrMachowski / Home-Assistant-custom-components-Xiaomi-Cloud-Map-Extractor

This custom integration provides a way to present a live view of a map for Xiaomi (Roborock/Viomi/Roidmi/Dreame) vacuums without a need for rooting.
MIT License
1.13k stars 122 forks source link

Split Map Extractor into a pypi package #418

Closed Lash-L closed 1 year ago

Lash-L commented 1 year ago

Description

Per our discussion here, it would be good to have the map extractor as a separate pypi package, so that other integrations/ non-home assistant integrations can use the logic that was created here.

Solution

Create a separate python repository that holds the logic for map extraction and then host it on pypi.

Alternatives

No response

Context

No response

Lash-L commented 1 year ago

Hey @PiotrMachowski let me know how can I help with this. People keep asking me about camera extraction in regards to the core integration, so if you'd like help with Thai I'd be happy to

Even if it is just setting up the repo with how the code is right now just so you have a basis to work with

humbertogontijo commented 1 year ago

@PiotrMachowski I'm also glad to help if needed

PiotrMachowski commented 1 year ago

At first I need to decide how to split the code into packages. There is a lot of shared code between different platforms, so it needs to be done in a nice way to avoid code duplication. My current idea is to make one common package and separate packages for every API that will use the common one. What do you think about it?

humbertogontijo commented 1 year ago

Sounds good to me

Lash-L commented 1 year ago

Logical to me as well. good for scaling and adding new apis in the future too

Lash-L commented 1 year ago

Saw you marked this as in progress, let me know if you need anything!

PiotrMachowski commented 1 year ago

@Lash-L I also try to make some refactoring as well. Do you have some suggestions?

Lash-L commented 1 year ago

Well this isn't necessarily a suggestion, but the core devs have asked me to try to get Maps directly into HA core as a new Entity. I was wondering if that is something you would be interested in as well? I'm currently working on adding rooms to the Vacuum entity as a kind of first step.

As far as this package goes, I think what you said earlier makes sense about the common package and splitting up code specific stuff. Or even just some basic subclassing. As far as any other specific suggestions go, I don't really have anything.

humbertogontijo commented 1 year ago

Making the common package would be nice, but if you want something easier, maybe just creating a CameraEntity receiving some interfaces would also work

Lash-L commented 1 year ago

@humbertogontijo how would he make the actual entity in the package since it's not within HA?

humbertogontijo commented 1 year ago

@Lash-L not actually implementing the CameraEntity but providing a class to be extended by one implementation

PiotrMachowski commented 1 year ago

Quick update: I have finished refactoring and separating code to different packages, now I have to use them in Map Extractor, test everything and publish the results

Lash-L commented 1 year ago

Awesome, thanks for the update!

PiotrMachowski commented 1 year ago

I have extracted 5 packages:

and created an early version of Map Extractor that uses them.

Please feel free to suggest any changes and improvements.

Lash-L commented 1 year ago

Perfect! I'll take a stab at implementing it in the custom Roborock component tomorrow and let you know if anything comes up

PiotrMachowski commented 10 months ago

@Lash-L Have you found some time to check out created packages?

Lash-L commented 10 months ago

Hey @PiotrMachowski I have a local branch adding it for the custom integration, but I still have a bit to do. I've been a bit focused on the core integration, and I can't add it to the core integration until the Floor Plan proposal gets approved. But I'm thinking this week I'll have a better idea of any pain points with integrating the packages. Sorry for the delay!

PiotrMachowski commented 10 months ago

I have seen your Floor Plan proposal, it seems to be a quite big architectural change for HA. Anyway, I'm not pushing you, I'm just curious if my solution fits your needs.

Lash-L commented 9 months ago

I have seen your Floor Plan proposal, it seems to be a quite big architectural change for HA

Definitely is - still optimistic I'll be able to get something done as Paulus seems interested in the concept.

But for now I am just adding it as an image to the core entity. But question for you - it is working well, but it seems like the roborock extractor does not set the names of the rooms. And the image parser doesn't have the draw_room_name function like the generic one does.

Am I missing something?

PiotrMachowski commented 9 months ago

Are room names included in rrmap file?

Lash-L commented 9 months ago

Not that I am aware of - but they can be gotten through another command. Perhaps just being able to pass in a dictionary to map from room ids to names? Or would it be better if I just manually draw the text afterwards? I'm not sure, still working my way around this package

PiotrMachowski commented 9 months ago

I thiiink you can pass them using texts parameter. But to be honest I don't really like drawing texts on the map - by default it uses a non-scalable bitmap font that doesn't support any special characters and I don't think HA contains other fonts that are available for pillow to use.