Prepare component for pull to hass
michaelmcarthur opened this issue · 16 comments
If anyone can share what needs to be done to get this component added to home assistant.
Code will have to be tidied up. All issues need to be resolved first.
Created Markdown
Still to add icon to markdown
There is not much to do. Please comply with the style guides at https://home-assistant.io/developers/development_guidelines/ they will be enforced. Run tox (https://home-assistant.io/developers/development_testing/) to avoid annoying fixes and prepare a pull request afterwards. A second pull request at https://github.com/home-assistant/home-assistant.github.io is needed for the documentation. Let me know if you get stuck.
Thank you for your support. I have started working on this. I couldn't get tox to install properly but I have installed the test suites. The code has passed the flake8 test. I'm on to the pylint test now. Am I correct in saying if I pass the following tests it is the same as passing Tox?
$ flake8 homeassistant/core.py
$ pylint homeassistant/core.py
$ pydocstyle homeassistant/core.py
$ py.test tests/test_core.py
I think yes. If you make the PR a buildbot/travis and hound-ci picks up your code automatically and does the checks again. All failures will be reported as reviews/comments. It's your task to update your branch/PR as long the bots report errors.
Do you need assistance?
I would love some help. The component is stable and works for all users. I started preparing it to submit but I struggled to tidy up the code. I managed to do flake8 but pylint caused me issues. I’m not a programmer so I ran into issues trying to adhere to a style. I used this as an opertunity to learn how to code and create a component. What assistance can you offer?
First off, let me say “congratulations” – for not being a programmer, you did an outstanding job with this!
I’m a long time Python developer and I’ve contributed several components to HASS (like AirVisual and RainMachine); because of that, I've been through all the pitfalls of submission. 😄
Some ways I might help:
- fix all of the styling issues that you mentioned
- submit PRs that achieve some of the other functionality and your issue backlog (if needed)
- help write the documentation
- create the official HASS PR on your behalf (or walk you through the steps if you’d like to do it yourself)
Happy to help in whatever way you feel is appropriate!
Thank you for your offer. It would be a huge help as I have little free time due to having a 4 week old baby to look after. He has taken up nearly all my free time.
Feel free to work on my code. It would mean a lot to me if the component made it on to home assistant. The issue back log is not too much of an issue as it just adds more functionality to the component. Let me know what the steps are about submitting a component as I would like to try are submit it myself.
Keep me updated how you get on. If you need me to test the code let me know and I’ll report back.
Totally understand. Happy to help. I'll be in touch!
Hi Michael,
Thanks for creating this component and making it available. It did everything I wanted except providing a place name.
I'm not a developer. I'm new to Home Assistant and to Python. Having said that - I used your existing code as a guide and extended your component to optionally retrieve a place_name and place_type from OpenStreetMaps. I'm sure I didn't do it "properly" and I probably (unintentionally) broke a number of development guidelines - but it works for what I wanted it to do - so, I intend to use it anyways.
On the off chance that it may also help others - I am submitting it for your review - and to "clean up" if you have a chance. The new functionality is enabled by adding 'place' to the existing OPTIONS string. It adds 2 new attributes - place_type and place_name.
If zone is 'not_home' and place_type is not 'house', it changes the 'sensor.state' to 'place_name'. Otherwise, it doesn't interfere with your original component at all. It just makes the 2 new attributes available for notifications, automations, etc.
If you do get a chance to review it, please let me know what you think. Thanks.
@tenly2000 nice work, thanks for your interest. It’s a request that lots of people want. I have very little time on my hands at the moment so have not been able to implement. I did work on an open street map version of the sensor. I just added it to the dev branch of google Geocode. You can use it by adding this to your sensor.yaml and adding the code to custom component sensor folder
- platform: openstreetmap_geocode
name: Donna
origin: device_tracker.michaelmcarthur_donna_loc
options: place
scan_interval: 30
It was made half way through working on google Geocode. It does work but it’s missing some of the features from the google version. It should be fairly easy to port over open street map to the Geocode version. Give it a go if you like tinkering with code. If I find some time this week I’ll try and port it over.
Hi @michaelmcarthur ,
Thanks for uploading to the dev branch - but for now, I think I'm going to stick with my modifications to your original google_geocode component. It has all of the benefits of your work with all of your features - and then simply makes an additional call to openstreetmaps to grab the place information. I'm assuming that there was a reason you originally chose google instead of openstreetmaps? Better accuracy? Better reliability with the google API?
I also don't have a lot of free time on my hands and there's still so much more to learn and do with Home Assistant. I don't want to tinker with code just for the sake of tinkering with code. I need to have a goal for doing so. I wish I'd known about your dev branch version before I started tinkering with this one - but since I put in the time with this one and have it working the way I want it to - I'm just going to use it for now. I will definitely keep an eye on your page here and if/when you finish up the openstreetmap version with places and all of the other features - and make it as full featured as your google component - I will certainly give it a try! I actually found a minor error in mine and tweaked it an hour ago (to account for type="None" as well as type="house"). Attaching the new version on the off chance you or anyone else cares.
What happened with getting your component built-in to Home Assistant? It looks like you were working towards that several months ago but it's still not there… Can I ask why it takes so long? I think a lot more people would use it if it were built-in rather than a custom component.
Thanks again…
@tenly2000 good work with the code. I suggest putting it on the google geocode home assistant forums page. I'm sure other people would be interested in what you have done. I'm not a coder myself but I like tinkering. This project was just a way to add a missing feature to home assistant and learn some code.
I would love to get it built in to home assistant, but I do not have the coding skills to format the code for submission. It's on the to do list to learn but I have a new born baby to look after and he takes up all my free time.
@tenly2000 you got me thinking about this project again. I merged the openstreetmap data with google geocode. The file in the dev branch has all the features of the google version it uses openstreetmap for the data. This means if you put places as an option it will display the place name.
Getting the place name is turning out to be a little more difficult than I thought. It seems that there is a lot of flexibility to the openstreetmap place database schema and the place name is not always stored in the same attribute. My code worked fine for the handful of test locations I tried it with - but now I'm seeing other "place_types" that fall through the rules I built. Not sure how extensive this is going to be, and I imagine it will be even worse for addresses in other countries. I'm going to have a look at your code to see how you handled it.
Were you able to find any kind of 'definitive' schema definition for the openstreetmap database?
I've deleted the file I uploaded yesterday. It turns out it's not ready to be shared.
Ok. I just looked at your code and see that you're just looking in ["namedetails"]["name"] for the place name. Here's an example of a place that doesn't work with that approach. Also, I see you're using "format=json" in your url. If you use "format=jsonv2" instead, you also get the "type" of place in the response which can be used to find the right attribute for the place name (["address"][place_type]) in many cases (but not the one below). Note that the name is stored in ["namedetails"]["addr:housename"] instead of "name" and the "type" should be equal to "building" which would let us use the ["address"][type] reference to find it - but it's not - it's just set to "yes".
For the wishlist - I'd also like this component to return "place_type" and "neighbourhood" (if/when it exists) as additional attributes.
2018-01-29 10:31:27 INFO (Thread-13) [custom_components.sensor.google_osm_geocode] openstreetmap request sent with lat=43.65260619739398and lon=-79.37842787882381
2018-01-29 10:31:27 INFO (Thread-13) [custom_components.sensor.google_osm_geocode] OSM DEBUG: {"place_id":"85405853","licence":"Data © OpenStreetMap contributors, ODbL 1.0. http://www.openstreetmap.org/copyright","osm_type":"way","osm_id":"61698598","lat":"43.65224015","lon":"-79.3785475623832","place_rank":"30","category":"building","type":"yes","importance":"0","addresstype":"building","display_name":"One Queen East, 1, Queen Street East, Financial District, Old Toronto, Toronto, Ontario, M5B 1W8, Canada","name":"One Queen East","address":{"building":"One Queen East","house_number":"1","road":"Queen Street East","neighbourhood":"Financial District","city_district":"Old Toronto","city":"Toronto","state":"Ontario","postcode":"M5B 1W8","country":"Canada","country_code":"ca"},"extratags":{},"namedetails":{"addr:housename":"One Queen East"},"boundingbox":["43.6519899","43.6525086","-79.3790365","-79.3780187"]}