Truncate route to 255 characters for here_travel_time
Proposed change
Alternative solution to https://github.com/home-assistant/core/pull/74348.
Strip route segments from the front until there are 255 or less characters.
A next step could be that the whole route is emitted as an event. Advanced users can still opt to create template sensors from them or use the full route as the seem fit.
Type of change
- [ ] Dependency upgrade
- [x] Bugfix (non-breaking change which fixes an issue)
- [ ] New integration (thank you!)
- [ ] New feature (which adds functionality to an existing integration)
- [ ] Deprecation (breaking change to happen in the future)
- [ ] Breaking change (fix/feature causing existing functionality to break)
- [ ] Code quality improvements to existing code or addition of tests
Additional information
- This PR fixes or closes issue: fixes #74301
- This PR is related to issue:
- Link to documentation pull request:
Checklist
- [x] The code change is tested and works locally.
- [x] Local tests pass. Your PR cannot be merged unless tests pass
- [x] There is no commented out code in this PR.
- [x] I have followed the development checklist
- [x] The code has been formatted using Black (
black --fast homeassistant tests) - [x] Tests have been added to verify that the new code works.
If user exposed functionality or configuration variables are added/changed:
- [ ] Documentation added/updated for www.home-assistant.io
If the code communicates with devices, web services, or third-party tools:
- [ ] The manifest file has all fields filled out correctly.
Updated and included derived files by running:python3 -m script.hassfest. - [ ] New or updated dependencies have been added to
requirements_all.txt.
Updated by runningpython3 -m script.gen_requirements_all. - [ ] For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
- [ ] Untested files have been added to
.coveragerc.
The integration reached or maintains the following Integration Quality Scale:
- [ ] No score or internal
- [ ] 🥈 Silver
- [ ] 🥇 Gold
- [ ] 🏆 Platinum
To help with the load of incoming pull requests:
- [ ] I have reviewed two other open pull requests in this repository.
@allenporter do you think this is a fitting intermediate solution instead of #74348 ?
As mentioned in the description I would open a architecture discussion how the full route might be utilised.
@allenporter do you think this is a fitting intermediate solution instead of #74348 ?
As mentioned in the description I would open a architecture discussion how the full route might be utilised.
Based on the response from the other core devs on the thread, this seems like it still falls in the workaround category.
In his last response @frenck wrote:
Meaning it is working around limitations. We should not settle for workarounds.
This PR would not work around the limitation by "abusing" the attribute functionality. It rather fulfills the limitation as designed.
If it still is a workaround and the only solution is to remove the information, shouldn't that than also be the case for waze and for information in entities of integrations like calendar?
Maybe one comment or use case from a simple user ;-) I filter the route into essential information, which differs per route:
- unique_id: joergs_way_to_work
attributes:
route: >
{% set list = states('sensor.jorg_arbeitsweg_route')|regex_findall('A[0-9]{1,3}', ignorecase=false)%}
{{list|replace('[','')|replace(']','')|replace("'",'')}}
unit_of_measurement: min
destination_name: "netgo Borken"
friendly_name: "Jörgs Arbeitsweg"
duration_in_traffic: "{{ states('sensor.jorg_arbeitsweg_duration') }}"
duration: "{{ states('sensor.jorg_arbeitsweg_duration') }}"
state: "{{ states('sensor.jorg_arbeitsweg_distance') }}"
I filtered everything except the 'Axxx'. The result is the list of highways, I have to use. This list gives me the indication in which direction I have to start and where I can stop for buying lunch.
for my wife I filter as follows:
- unique_id: yvonnes_way_back_home
attributes:
route: >
{% set list = states('sensor.yvonnes_heimweg_route')|regex_replace('L[0-9]{1,4}', ignorecase=false)%}
{{list|replace(' ','')|replace('-',' ')|replace("'",'')}}
unit_of_measurement: min
destination_name: "Willicher Straße"
friendly_name: "Yvonnes Heimweg"
duration_in_traffic: "{{ states('sensor.yvonnes_heimweg_duration') }}"
duration: "{{ states('sensor.yvonnes_heimweg_duration') }}"
state: "{{ states('sensor.yvonnes_heimweg_distance') }}"
She does not neet the information 'L233 - Pestalozzistrassse' she only needs the streetname. So we filtered everything which beginns with 'Lxxx'. For her it is essential as well because she has 3 to 4 possible routes and wants to know upfront where to go
If I had to open google maps or another solution first, the here integration is some kind of useless for me cause the integration saves exactly this action in the morning. Alexa tells me the Highways and I know everything I need: when to start and the way I have to use.
My preferation is the possibility to configure the Route and define the information I need per entity.
@frenck could you please give your opinion whether this approach is feasible?
Based on the response from the other core devs on the thread, this seems like it still falls in the workaround category.
I agree with that.
Strip route segments from the front until there are 255 or less characters.
This kinda amplified that. It ensures it will meet the technical requirements, at the cost of just throwing out data. That shows both seeking to work around it and it kinda shows the data is less important than it seems.
I'm going to friendly decline the addition of this sensor to Home Assistant.
../Frenck
@frenck this isn't an addition of a sensor. This is a sensor which is released. The issue this PR is trying to solve came up when I split up the single sensor. The route information is a part of Homeassistant since release 0.100.
There where several people in the issues/prs and the forum to which I linked showing how they are using the route information and also giving a reason why truncating the front of the route still preserves the important information.
Can you please help me understand how this helps the users of homeassistant?
I currently see this as a punishement of the users of this integration because I was trying to improve it. If I hadn't split up the sensor and leave it be we wouldn't be missing functionality.
Could you please try to explain you reasoning in another way. I am trying to understand the guidelines for future contributions and currently my understanding is, that you dont like this PR. The fact that you argue, that this PR adds a sensor, which it does not at all, also makes the impression on me that you don't care enough.
You said about yourself in several interviews, twitter posts etc. that you are often misunderstood and I do think that you have a good reason for your decision.
Something along the lines of
We should use data in homeassistant provided by services/devices as is or not at all. If technical or any other reasons limit the amount of data homeassistant can use/show it should not be included. The nature of the route information should have been detected earlier and we must remove the information alltogether. The use cases layed out by the different users do not outweigh the drawbacks which are..... would really help.
Thank you.
Could you please try to explain you reasoning in another way.
It has been explained a couple of times before. The addition has been discussed with other core developers and we won't add this specific data back into any sensor, state, or attribute.
also makes the impression on me that you don't care enough.
We do, the sensor needs to be removed to fix the issue.
ah this is truly bad news. Hope it wont trickle down to taking out other useful information to the Home, like MeteoAlarm or likewise sensor attributes:
instead of simply taking out those longer attributes (or creating standalone sensors) because of some arbitrary logic (how many characters would be acceptable to record?), I would hope some discussion could be started on how to alternatively satisfy the need for that essential Home info.
Only stating users can open a dedicated app is not very helpful really. We can do that with many integrations, and the power of HA is exactly that we dont need to open other apps, even though they are available.
Hope it wont trickle down to taking out other useful information to the Home, like MeteoAlarm or likewise sensor attributes:
Yes, it will, in the end, all these things have the same issue.
Also, that integration is not related to this PR. Let's keep PRs on the topic of the integration in the PR.
ofc. sorry bout that. My example was not about that integration though, but more of an illustration.
Please let's focus on how the Project can keep that info, in a sane way for the project, instead of killing it because of current limitations?