Refactoring command function to make result handling and exception handling easier

[markprins]

In the HomeAssistant code I have to catch exceptions every location a command is called. Right now that is due to the calling of either command() or power_state().

They both trigger _send_command() but in a different way.

I wanted to refactor the command() so it can handle both commands that do return results or commands that don't return results:

A basic version was working, but it meant changing a lot of the basics and it felt like I was adding spaghetti-code and magic behaviour.

In essence I altered command() to check if the provided command_string was a ResultCommand or a Command. In case of a ResultCommand it needs an ack, which required knowing which ack to receive. For power state this is easy and with a minor change using the ACKs enum to get the ack based on the command string was possible. However imput changes are variable, but the ack the same. So that would mean introducing multiple ACKs with same command like so:

class ACKs(Enum):
    power_status = b"\x06\x89\x01\x50\x57\x0A"
    hdmi1 = b"\x06\x89\x01\x49\x50\x0A"
    hdmi2 = b"\x06\x89\x01\x49\x50\x0A"

It worked okay-ish. But after the comand was sent, the result must be handled and stored. I didn't want to alther the return True/False on command so extendend it to save the message, which could then be processed in a helper function for that power status. I ended in finding if a _translate_<command>() function-ish situation which did work. But as said, it felt spaghetti-code like.

The command looked something like this:

    def command(self, command_string, receive=False):
        print(command_string)

        if hasattr(Commands, command_string):
            self._send_command(Commands[command_string].value)
            return True

        if hasattr(ResultCommands, command_string):

            result_message = self._send_command(ResultCommands[command_string].value, ack=ACKs[command_string].value)
            if hasattr(self, '_translate_' + command_string) and callable(getattr(self, '_translate_' + command_string)):
                self.result_message = getattr(self, '_translate_' + command_string)(result_message)

            return True

        return False

Right now it feels like there is an extra layer of information is required which maps commands to results or a way of interpreting results. Maybe a grouping of certain commands and its generic handling (like that input command results should be handled the same) and that certain commands are ACK-ed the same.

If this is refactored, adding of the commands (which sroelens asked about in #4) is then possible.

Food for thought!

[bezmi]

It is possible to implement all of the commands in #4 as long as we don't care about the returned message. IMO, for most "remote" type commands, which is what this module was designed for, that's fine. For my use case, I only really need to know the power state of the projector. Being able to fetch other information was cool but not worth the headache.

For a more fully featured implementation, see arvehj/jvcprojectortools, specifically jvc_command.py.

[bezmi]

Closing as the latest versions of this library have got better exception handling