Implement virtual device support #139
Conversation
|
I think especially the modifications I've done in In general the handling of a virtual device in F@H is exactly 100% reversed to the handling of "normal" devices. This means to change the state from the outside we need to write to output-datapoints and changes from F@H are written to input-datapoints where we need to react on (callbacks). |
|
The communication flows are as follow:
A virtual device in F@H is fully controlled by an external program, so modifications by an external program are treated as fact in F@H. If F@H wants to modify a virtual device it sends a "wish" to the external program and the external program needs to aknowledge this wish (if not, the state change in F@H is reversed) |
There was a problem hiding this comment.
Ideally we try to keep the virtual device logic completely out of the existing devices classes. Maybe having another directory virtual_devices including a new base class with the existing base class as the parent. Then only adding in the logic required (or changing existing logic e.g. refresh_state() for virtual devices.
I think once we start mixing logic in existing classes it'll start to blur the lines between a normal device and a virtual device.
My review of this is a little bit all over the place, I realize that.
| @@ -16,4 +16,3 @@ class Interface(enum.Enum): | |||
| WIRELESS_RF = "RF" | |||
| HUE = "hue" | |||
| SONOS = "sonos" | |||
| VIRTUAL_DEVICE = "vdev:[email protected]" | |||
There was a problem hiding this comment.
Because this is technically supposed to be there I'd keep it. Maybe ABB will implement a fix if we bring it to them.
There was a problem hiding this comment.
After first tests within the HA-integration I also found at that it is needed as - at least in my setup - I have some virtual devices with the interface-setting and some without (UNDEFINED).
So at the moment e.g. for HA I need to include Interface.VIRTUAL_DEVICE and Interface.UNDEFINED and additionally check for the serial-number. 😢
| for _pairing in self._state_refresh_input_pairings: | ||
| _input_id, _input_value = self.get_input_by_pairing(pairing=_pairing) | ||
|
|
||
| _datapoint = ( | ||
| await self._api.get_datapoint( | ||
| device_id=self.device_id, | ||
| channel_id=self.channel_id, | ||
| datapoint=_input_id, | ||
| ) | ||
| )[0] | ||
|
|
||
| self._refresh_state_from_input( | ||
| input={ | ||
| "pairingID": _pairing.value, | ||
| "value": _datapoint, | ||
| } | ||
| ) |
There was a problem hiding this comment.
I don't think we want to apply this logic to all devices as it's only required for virtual devices. I think a self._is_virtual_device attribute makes sense. Then only running the refresh state from the api for either inputs, or outputs, depending on if it's a virtual device.
Completely different, I'd probably rename this to refresh_state_from_api.
There was a problem hiding this comment.
I will think about it. Also if we will introduce an additional base class, perhaps some of the logic can be moved to it and makes it more clearer
| FUNCTION_DEVICE_MAPPING_VIRTUAL: dict[Function, Base] = { | ||
| Function.FID_WINDOW_DOOR_SENSOR: WindowDoorSensorVirtual, | ||
| Function.FID_SWITCH_ACTUATOR: SwitchActuatorVirtual, | ||
| } |
There was a problem hiding this comment.
Do we need another variable for this? I'd probably keep this within the FUNCTION_DEVICE_MAPPING variable. I think adding another variable adds some additional complexities.
There was a problem hiding this comment.
I don't know how else you want to make the distinction between the "Function Device Mapping" of normal devices and the "Function Device Mapping" of virtual devices (they share the same functionIDs but need different classes). As you see in my implemented examples with this setup we can easily control which virtual devices we support.
| # Filter out virtual devices | ||
| if _device_key[0:4] == "6000" and not self._include_virtual_devices: | ||
| continue |
There was a problem hiding this comment.
Is there any documentation that supports all virtual devices will start with 6000?
I'm absolutey happy to split this up. I'm not sure if also a dedicated directory is needed. If we just create a new base-class for the virtual devices e.g. |
|
I will close this pull-request and start from scratch. |
This is the first try to implement virtual device support. No tests are written yet as I first want to have consensus about my approach.
I re-implemented everything what I removed in #103 and created dedicated ...Virtual classes.
For now I only implemented a "WindowDoorSensor" and a "SwitchActuator".
As the return of the F@H-API regarding virtual devices
interface=vdev:...seems to be unreliable I moved to a check of the device_id: All my tests showed that a virtual device device_id always starts with6000