Skip to content

Add Home Assistant device tracker#5701

Closed
mueslo wants to merge 1 commit into
openwrt:masterfrom
mueslo:master
Closed

Add Home Assistant device tracker#5701
mueslo wants to merge 1 commit into
openwrt:masterfrom
mueslo:master

Conversation

@mueslo
Copy link
Copy Markdown

@mueslo mueslo commented Mar 2, 2018

Maintainer: me
Compile tested: LEDE Reboot 17.01.2 r3435-65eec8bd5f
Run tested: LEDE Reboot 17.01.2 r3435-65eec8bd5f, tested locally

Description:
New package that notifies Home Assistant on hostapd events for presence detection. See also: https://github.com/mueslo/openwrt_hass_devicetracker

Comment thread net/hass/files/hass.conf Outdated
@@ -0,0 +1,5 @@
config hass 'global'
option host '1.2.3.4:8123'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if you must do this, use something.example.org instead of 1.2.3.4

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I think a local-only IP would be better than a DNS name that could be registered at some point, but yeah, 1.2.3.4 is a bad idea.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No, I literally meant "something.example.org" example.{com,org,net} are permanently reserved: https://tools.ietf.org/html/rfc2606 no-one can register them :)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

TIL, thanks!

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The TLD example. is also reserved, so I used that.

Comment thread net/hass/files/hassd.sh

logger -t $0 -p info "Starting up"

source /usr/lib/hass/functions.sh
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

you've "" quoted above, but not here. I don't care which you use, but consistency is nice.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

you're right, just copied the upper part everywhere.

@dibdot
Copy link
Copy Markdown
Contributor

dibdot commented Mar 4, 2018

A complete non-technical feedback regarding your chosen package name "hass", an ugly german word which means "hate"/"hatred". In times of Trump, alternative facts and hatred of foreigners not a good choice ... just my very personal reflection as I first saw your PR.

@karlp
Copy link
Copy Markdown
Contributor

karlp commented Mar 4, 2018

@dibdot it's not this packager's choice, it's home assistant itself. => hass.io "Home ASSistant" I'd say using the "hass" name is most consistent and least surprising for the user.

@dibdot
Copy link
Copy Markdown
Contributor

dibdot commented Mar 4, 2018

@karlp thanks for clarification, still I find this name quite awful (as a german native speaker).

@mueslo
Copy link
Copy Markdown
Author

mueslo commented Mar 4, 2018 via email

Signed-off-by: Johannes Falke <johannesfalke@googlemail.com>
@disrupted
Copy link
Copy Markdown

any news on this?

@nstrelow
Copy link
Copy Markdown

Would like to see this integrated 😁

Comment thread net/hass/Makefile
define Package/hass
SECTION:=net
CATEGORY:=Network
TITLE:=Wireless device tracker for Home Assistant
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ok, I missed this earlier. If this package is just the wireless device tracker, and not Home Assistant itself, then Shouldn't the package name be something like "hass-device-tracker" or something? not just "hass" itself?

Comment thread net/hass/files/hass.conf
@@ -0,0 +1,5 @@
config hass 'global'
option host 'ip.or.name.example:8123'
option pw ''
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if that's password, please just use password, instead of making people guess.

Comment thread net/hass/files/hass.init
stop_service() {
for pid in $(all_child_pids $(cat $PIDFILE)); do
kill $pid
done
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what did you do that procd isn't handling this for you? consider fixing your scripts that you don't need this sort of procedure to die politely.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The way I'm spawning processes, procd does not correctly kill all child processes, that's why I wrote this.

Comment thread net/hass/files/hass.init
reload_service()
{
stop
start
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

99% sure you don't need this, as you're doing the default implentation.

@nstrelow
Copy link
Copy Markdown

@karlp Thanks so much for pushing this forward 🥇

@mueslo
Copy link
Copy Markdown
Author

mueslo commented Nov 24, 2018

Thanks for the feedback, I'll address those issues and comment back here when I think it's actually ready for merging.

@nattofriends
Copy link
Copy Markdown

Any update on this?

@feckert
Copy link
Copy Markdown
Member

feckert commented Sep 6, 2019

@mueslo ping any progress on this?

@ohadlevy
Copy link
Copy Markdown

bump please? :)

@castillofrancodamian
Copy link
Copy Markdown

The most forgotten 😅😅😅

@BKPepe BKPepe closed this Apr 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.