Input from group?

[maximilianriemensberger]

GroupConnection and ModuleConnection both derive from AbstractConnection. The AbstractConnection implements handling of inputs from the bus and forwards it to input callbacks. However, is it actually possible to receive input from a group? If not, shouldn't all of the inputs handling be moved from AbstractConnection into ModuleConnection?

The same arguments apply to the request handlers. That is, activate_status_request_handler(s) should not be needed in GroupConnection.

[alengwenus]

Indeed, async_process_input and register_for_inputs could be moved to ModuleConnection as it's not possible for groups to receive status messages. But then we might need case differentiations at least in PchkConnectionManager (and also maybe in HomeAssistant code). I have to check that...

For the StatusRequestsHandler I think of moving it to AbstractConnection. It should be possible to send status requests to groups. As far as I remember the StatusRequestsHandler only emits status requests at certain intervals. It does not handle any status responses (except one method which is used to handle untyped variable responses in a different context). The responses are all handled by predefined callbacks for the corresponding ModuleConnection. Paying attention to some side effects (e.g the typeless variable lock), I guess we even could move the StatusRequestsHandler to AbstractConnection?

[maximilianriemensberger]

Indeed, async_process_input and register_for_inputs could be moved to ModuleConnection as it's not possible for groups to receive status messages. But then we might need case differentiations at least in PchkConnectionManager (and also maybe in HomeAssistant code). I have to check that...

I have a branch that does that and also handles both types of connections differently. I feel that it looks cleaner at least inside pypck. I haven't checked with HomeAssitant so far.

For the StatusRequestsHandler I think of moving it to AbstractConnection. It should be possible to send status requests to groups. As far as I remember the StatusRequestsHandler only emits status requests at certain intervals. It does not handle any status responses (except one method which is used to handle untyped variable responses in a different context).

I don't know. If I look at the current impl of activate_status_request_handler in GroupConnection, it doesn't do anything except waiting for the segment scan. So I suppose nobody should have used it so far with aGroupConnection. I agree thatStatusRequestHandlercould be used on a group as well provided there are no typeless variables. The typeless variables preprocessing will be heavily messed up if anybody usesStatusRequestHandleron a group because the response will go to theModuleConnectionsand those don't know what was the last requested var. My suggestion would be to move theStatusRequestHandlertoModuleConnectionor alternatively use it inGroupConnection` in a way that does not ask for typeless variables and therefore does not do any input processing.

The responses are all handled by predefined callbacks for the corresponding ModuleConnection. Paying attention to some side effects (e.g the typeless variable lock), I guess we even could move the StatusRequestsHandler to AbstractConnection?

Yes I think it's possible, but I would do that in a separate step since as far as I can tell it's currently unused on GroupConnection. So I think there low change of breaking user code if the activate_* functions in AbstractConnection are stubbed out properly. And probably even deprecated.

Another effect that I noticed is that if no request handlers are used on GroupConnections, they become more or less stateless. This means you can create and destroy them as need as they are just used to send commands and do not expect responses. This makes PchkConnectionManager only hold references to ModuleConnections and thus for stronger typing when interacting with stored connections. (Most of the interaction except sending commands require a ModuleConnection and can't be done with anAbstractConnection or GroupConnection.)

I'll send a PR as basis of discussion shortly.

[maximilianriemensberger]

Also request_serials and serial_known can be called on both a GroupConnection (via AbstractConnection) and a ModuleConnection. However, the AbstractConnection implementation doesn't do anything useful. And in particular for request_serials it does not even issue the request which is a bit misleading. So I would in a first step remove both from AbstractConnection as well. Proper request functionality for the group can then be added again in a second step in my opinion.

OTOH its heavily used inside AbstractConnection. For the GroupConnection it just doesn't wait for the event while in the ModuleConnection case it does wait. So it's not as easy to remove. But the associated functionality could be made internal to AbstractConnection to avoid exposing those stubs.

[alengwenus]

I thought about it for a while. I agree, it makes no sense to have the StatusRequestHandler in the GroupConnection and we should remove it. Instead we should add a convenience method to PchkConnectionManager to get references to all ModuleConnections belonging to a specified group (using the LEER command). Probably we also can use the scan_modules command and just add an optional group-parameter. The references can then be used to eventually activate the module`s StatusRequestHandlers manually.

[maximilianriemensberger]

I thought about it for a while. I agree, it makes no sense to have the StatusRequestHandler in the GroupConnection and we should remove it. Instead we should add a convenience method to PchkConnectionManager to get references to all ModuleConnections belonging to a specified group (using the LEER command). Probably we also can use the scan_modules command and just add an optional group-parameter. The references can then be used to eventually activate the module`s StatusRequestHandlers manually.

I like the idea.

BTW. I have run into problems with the LEER command scanning: It misses modules even after retries. I experience that there are a few modules that are likely to fail the LEER scan if the group is large (e.g. broadcast, multiple floors combined). So I'm contemplating how to make the topology scanning and also group membership association reliable or at least compare it to a well-known state. But I think that's a problem for another day :smirk:.

[alengwenus]

Interesting... I opened an issue for that: #67