Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiple devices #146

Closed
maaraneasi opened this issue Sep 9, 2019 · 12 comments · Fixed by #157
Closed

Multiple devices #146

maaraneasi opened this issue Sep 9, 2019 · 12 comments · Fixed by #157

Comments

@maaraneasi
Copy link

maaraneasi commented Sep 9, 2019

Hey all!
I am so happy I found this project!!! :-)
I don't have exactly "an issue" but rather a very noob question, need small help and don't know where else to ask...
I am trying to create multiple accessories at the same time as it seems to be supported by the ip transport:
"The transports can contain more than one accessory. If this is the case, the first accessory acts as the HomeKit bridge."
...yet I got stuck here...
I have a map of the accessories (my code needs to store the device name and its unique address I am passing to the ON/OFF commands which communicate further via telnet), but I am not sure how to properly construct multiple accessories from this map and pass all the accessories to the transport...
My idea is to build a slice and pass it to the transport, yet the is the place where I got unsure:

  1. The example creates acc := accessory.NewLightbulb(info), but what if I have more light bulbs? Should I create a slice and append all these acc's (if so then of what type this slice should be?)?
    I tried to create a slice var lampSlice []*accessory.Lightbulb and fed my custom lightbulbs into this slice, yet I can't use this slice in NewIPTransport() and this function expects *accessory.Accessory.......
  2. how to handle functions like acc.Lightbulb.On.OnValueRemoteUpdate(func(on bool) {} which are created for the accessory which are created for the specific accessory?

Is there an example of creating multiple accessories I could check and consult with?
Thanks a lot!

@brutella
Copy link
Owner

brutella commented Sep 9, 2019

  1. The example creates acc := accessory.NewLightbulb(info), but what if I have more light bulbs? Should I create a slice and append all these acc's (if so then of what type this slice should be?)?
    I tried to create a slice var lampSlice []*accessory.Lightbulb and fed my custom lightbulbs into this slice, yet I can't use this slice in NewIPTransport() and this function expects *accessory.Accessory.......

If you have a map of light bulb accessories, you have to convert it to a slice of generic accessories like this.

var lights map[string]*accessory.Lightbulb
var accessories []*accessory.Accessory
for (string, light) := range lights {
    accessories = append(accessories, light.Accessory)
}

Then you can create an ip transport with the accessories slice.

  1. how to handle functions like acc.Lightbulb.On.OnValueRemoteUpdate(func(on bool) {} which are created for the accessory which are created for the specific accessory?

I would create a function which takes the telnet address and returns a light bulb accessory.
Inside this method you create an accessory and implement the acc.Lightbulb.On.OnValueRemoteUpdate(func(on bool) {}) callback.

There you have all the information you need to implement the two-way communication between telnet and HomeKit.

@maaraneasi
Copy link
Author

Will give it a try! Thanks for help and thanks for such a great implementation (and sorry for my questions - this is one of my early attempts writing in golang...)

@maaraneasi
Copy link
Author

Seems I panicked prematurely (and deleted my previous comment) :-) My issue is that I am passing too many devices to the transport - is there any limitation? - I was trying to send about 150 devices to the transport (all the relays on the control unit) what seems too many. I went down to 4 and it works fine (almost). The only issue I have is that I am able to control only one of the created lightbulbs (which is the very first one I turn on after starting the program)... If I check the phone app, I see the devices having the correct SN (which is the telnet address) but any ON / OFF call is reusing the address of the first device I turned on....

I am setting the on / off action in the same loop where I am creating the accessory object:

for _, object := range objectmap {
		if object != nil {
				log.Printf("Building object: &s", object)
				accInfo := accessoryInfoFiller(object.address, object.alias)
				log.Printf("Info Struct2: %+v\n", *accInfo)
				acc := accessory.NewLightbulb(*accInfo)
				log.Printf("Accessory object : &s", &acc)
				log.Printf("Accessory object2: %+v\n", *acc)
				log.Printf("Accessory object3: %+v\n", acc)
				acc.Lightbulb.On.OnValueRemoteUpdate(func(on bool) {
					if on == true {
						turnLightOn(object.address)
						log.Println("Action ON: %v", object.address )
					} else {
						turnLightOff(object.address)
						log.Println("Action OFF: %v",  object.address)
					}
				})
				lights[object.address] = acc

...and where I am filling in the accessory info (which is correct for each object)

func accessoryInfoFiller(serialNr string, accName string) *accessory.Info {
	i := accessory.Info{Name: accName}
	i.SerialNumber = serialNr
	i.Manufacturer = "Test device"
	log.Printf("Info Struct: %+v\n", i)
	return &i
}

The ON/ OFF action is the one from the example and it expects
ie.

func turnLightOn(deviceAddress string) {
	log.Println("Turn Light On:")
	log.Println("ON command: %s", "SET,"+deviceAddress+",1")
	Telnet_connect("SET," + deviceAddress + ",1")
}

The problem is that if I toggle the device (for example) 0x01020027, all the other ON / OFF actions will be using the same device SN instead of SN of the accessory I am trying to control:

2019/09/10 11:48:00 Turn Light On:
2019/09/10 11:48:00 ON command: %s SET,0x01020027,1
2019/09/10 11:48:00 Calling telnet with %s SET,0x01020027,1
2019/09/10 11:48:00 Action ON: %v &{0xc0001100d8}
2019/09/10 11:48:05 Turn Light On:
2019/09/10 11:48:05 ON command: %s SET,0x01020027,1
2019/09/10 11:48:05 Calling telnet with %s SET,0x01020027,1
2019/09/10 11:48:05 Action ON: %v &{0xc0000c2060}
2019/09/10 11:48:13 Calling telnet with %s SET,0x01020027,0
2019/09/10 11:48:13 Turn Light Off:
2019/09/10 11:48:13 OFF command: %s SET,0x01020027,0
2019/09/10 11:48:13 Action OFF: %v &{0xc0000c2060}
2019/09/10 11:48:17 Calling telnet with %s SET,0x01020027,0
2019/09/10 11:48:17 Turn Light Off:
2019/09/10 11:48:17 OFF command: %s SET,0x01020027,0
2019/09/10 11:48:17 Action OFF: %v &{0xc0001100d8}

creating the accessory:

2019/09/10 11:47:49 Building object: &s%!(EXTRA *main.cuobject=&{RE2OBYVAKPODHLED 0x01020026 0x00000000  1 2})
2019/09/10 11:47:49 Info Struct: {Name:RE2OBYVAKPODHLED SerialNumber:0x01020026 Manufacturer:Test device Model: FirmwareRevision:}
2019/09/10 11:47:49 Info Struct2: {Name:RE2OBYVAKPODHLED SerialNumber:0x01020026 Manufacturer:Test device Model: FirmwareRevision:}
2019/09/10 11:47:49 Accessory object : &s%!(EXTRA **accessory.Lightbulb=0xc0000c20d8)
2019/09/10 11:47:49 Accessory object2: {Accessory:0xc0000c6180 Lightbulb:0xc0000a4270}
2019/09/10 11:47:49 Accessory object3: &{Accessory:0xc0000c6180 Lightbulb:0xc0000a4270}
2019/09/10 11:47:49 Building object: &s%!(EXTRA *main.cuobject=&{RE3OBYVAKKRB 0x01020027 0x00000000  1 2})
2019/09/10 11:47:49 Info Struct: {Name:RE3OBYVAKKRB SerialNumber:0x01020027 Manufacturer:Test device Model: FirmwareRevision:}
2019/09/10 11:47:49 Info Struct2: {Name:RE3OBYVAKKRB SerialNumber:0x01020027 Manufacturer:Test device Model: FirmwareRevision:}
2019/09/10 11:47:49 Accessory object : &s%!(EXTRA **accessory.Lightbulb=0xc000110088)
2019/09/10 11:47:49 Accessory object2: {Accessory:0xc0000b6540 Lightbulb:0xc0000b8e40}
2019/09/10 11:47:49 Accessory object3: &{Accessory:0xc0000b6540 Lightbulb:0xc0000b8e40}
2019/09/10 11:47:49 Built: &s objects%!(EXTRA map[string]*accessory.Lightbulb=map[0x01020025:0xc0000a2250 0x01020026:0xc0000a2370 0x01020027:0xc0001168d0])

(sorry for the formatting of the debug prints and my terrible way of coding - I didn't care about the logs so far and all the pointers and types are still very confusing for me).
Any idea where did I screw it?

@bjanders
Copy link

My issue is that I am passing too many devices to the transport - is there any limitation? - I was trying to send about 150 devices to the transport (all the relays on the control unit) what seems too many.

The HomeKit Accessory Protocol (HAP) specification sets a limit of 150 accessories per bridge.

@maaraneasi
Copy link
Author

Thanks for confirming - I thought that I might be too demanding by not filtering out what I really need and passing all the relays to the protocol :-)
I am currently testing with 4 devices and it works fine (mostly). What is weird is that sometimes, when I run the program, it mixes the device addresses so the homekit app switches turn on random devices (although reliably and consistently). Checking the device address in the app (I can see the device SN in the device details), it's all correct yet the on/off actions are using a different SN they took "somewhere", so I had to stop the app, restart and it eventually restarts with the correct device name and sn binding. When checking by debug output, it builds the devices with the correct addresses so not sure how it gets so mixed up

@bjanders
Copy link

bjanders commented Sep 29, 2019

Devices (accessories) in HomeKit are identified by an ID. hc assigns the ID incrementally in the order you add them in the NewIPTransport() call. So if the order of your devices changes when you start up your application, then they will get mixed up in the Home app. This is a problem if you later add new devices to hc. For example, I initially started with a bridge (ID 1) and two thermostats (IDs 2, 3). I then added five window shades (IDs 4-8). Then I added a third thermostat. In my app I first add all thermostats and then all shades. Now, when I added the third thermostat it got ID 4, and all shades got an ID that was incremented by one compared to the initial setup (IDs 5-9), and things were mixed up in the Home app. It showed a sixth shade (ID 9) and the third thermostat was not visible, because the Home app thought that ID 4 was a shade. I think it would be better if one could assign the IDs yourself in hc, so that you could keep a consistent numbering, for example as part of the accessory.Info struct.

@brutella
Copy link
Owner

@bjanders That's a good point. Pull requests are always welcome. 😉

@bjanders
Copy link

bjanders commented Oct 1, 2019

I did a quick hack for my own purposes, but if there is chance for a proper change to get merged then I'll try to get it done. I'd make it so that it is backward compatible so that if you don't provide a custom ID then one would be assigned incrementally as it is done now. If you assign a custom ID I'd add a check that the ID is not already assigned to some other accessory.

@maaraneasi
Copy link
Author

hc assigns the ID incrementally in the order you add them in the NewIPTransport() call.

This sounds like the tricky part. I am loading my devices from a text file (which is being produced by my smart home control unit). I am currently having about 25 lights I am feeding into the transport and loading them from a slice changes their ID if I restart the app so its impossible to restart the app without deleting everything and adding all the devices in the ios app again.

@brutella
Copy link
Owner

brutella commented Dec 9, 2019

We could add an ID property to accessory.Info and use that if specified.
Any thoughts on this?

@mologie
Copy link

mologie commented Jan 25, 2020

I second adding an ID property to accessory.Info. Implementing dynamic bridges, where accessories are added after the fact or may disappear when decomissioned, is not possible otherwise.

This is also a good place to change the ID type to an int64: Currently, a 32-bit int is used, but the HAP spec (section 2.6.1) allows 64-bit instance IDs for IP accessories. Switching to a 64-bit ID would enable trivial mapping to IDs for bridges, e.g. by hashing an object-unique identifier and using that as ID.

I misremembered. The ID is already a 64-bit integer. I misremembered, but not completely! Phew! The ID should be an unsigned 64-bit integer, whereas it currently is a signed 64-bit integer.

@brutella
Copy link
Owner

brutella commented Jan 27, 2020

Thanks, those changes are now available in the master branch.
There is now a section in the readme to learn more about multiple accessories.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants