hass-agent / HASS.Agent

Unofficial development project for the HASS.Agent platform.
https://hass-agent.io
MIT License
393 stars 12 forks source link

Feature: Location sensor show as device tracker #138

Closed denisabt closed 2 months ago

denisabt commented 3 months ago

Instead of returning location info as text only, return a device_tracker location object, which can be used on a map

amadeo-alex commented 3 months ago

Would it be possible to use at least https://github.com/hass-agent/HASS.Agent/tree/development-2.1.0-beta3 as a base? Main is quite behind in the development - for example "friendlyName" not existing anymore in the backend code with exception of UI code.

amadeo-alex commented 3 months ago

We also might need to think about people already using this sensor - for them this is going to be a breaking change since the state is always "on" instead of the previous combination of values.

denisabt commented 3 months ago

This makes absolute sense, both of it @amadeo-alex - how could I not have thought of current users. I will work on that, either make the change non-breaking (either by adding a setting (default - off) to turn on to enable location-based response, or migrate into a second/new sensor, what do you think ?). Will do, will also use the latest code base, and commit once done.

denisabt commented 2 months ago

@amadeo-alex - what do you think - i was thinking to add a SensorsMod Form checkbox like cbResultAsLocation or similar (or a 'CbSetting1' - 'Sensor specific checkbox', unchecked by default (thus keeping default behaviour as it was), and only if checked, return a location result. This would allow existing users to keep their functionality with the sensor, and upon adding a new Sensor / modifying existing location sensor to switch to location result behaviour. Or - and that's my request for comment - create an additional GeolocationSensor class with the new behaviour. But that seems a lot of duplication to me. What's your opinion? I tend to prefer the first approach, but looking forward to what you think. tnx !

amadeo-alex commented 2 months ago

Sorry for being less than responsive, I have some life stuff that requires 120% of my attention :D Why closed? I wanted to have a closer look next week at what you've wrote.

denisabt commented 2 months ago

No worries at all! i dove into options yesterday, and decided to scrap the branch, not try to do it with AdvancedSettings, Migration, Query or anything of that - but to create a new Sensor (DeviceTrackerSensor), instead of adding it into the current sensor. That way, the current Geolocation sensor will keep behaving as is (returning a string with lat/lon), and the new LocationTrackerSensor (returning a location-conform resultset) can be migrated to / or used in addition, to display location on a map in home assistant. Seems cleaner to me and leaves all choices to the user and their requirement. Almost finished with the code (except texts), will push a new branch/pr soon :-)