Do not add analog input four on Automation pHAT to avoid warnings

[StefanZ8n]

To avoid problems on using automationhat.analog.read() when on an Automation pHAT, do only initiate analog input four when on a HAT.

I had problems with the node-red-contrib-automation-hat module always disconnecting/connecting because of these error messages and think it's good to solve this at the root.

[Gadgetoid]

Thank you!

Do you recall what error messages you were seeing? I would have thought a read from the ADC on channel 4 would work on Automation pHAT, since the ADC is the same it just doesn't (IIRC) have that channel wired to anything.

[StefanZ8n]

You're welcome. Unfortunately I can't remember the error exactly. I wasn't using automation-hat itself but the Node Red node-red-contrib-automation-hat which gave me an error on reading all analog input (automationhat.analog.read()). But I think I saw a value for analog 4 (always the same - 90 or 89 or so - can't remember).

[StefanZ8n]

Any chance to get this merged soon? :+1:

[shortbloke]

@StefanZi I agree this PR looks like a good idea. Better to not throw this error downstream for client apps to handle. Without this PR even the example input.py throws a warning when run against a pHAT:

./input.py
{'three': 0, 'two': 0, 'one': 0}
/usr/lib/python2.7/dist-packages/automationhat/__init__.py:117: UserWarning: Analog Four is not supported on Automation pHAT
  warnings.warn("Analog Four is not supported on Automation pHAT")

I believe the latest version (v0.2.0) of my node-red-contrib-automation-hat now handles this error being triggered, without this PR being accepted.

[Gadgetoid]

Okay I feel like some explanation as to why I haven't merged this is due-

Basically it calls is_automation_phat() in the main body of the module, so this gets run upon import. This then calls setup() which (true to its name) sets up all the GPIO, i2c, etc of Automation HAT.

The trouble is- the code in setup() is in there, rather than the main module body, specifically to avoid it being called at import time, so this module can be imported safely without causing import-time side-effects. This is specifically because certian Python tools will import modules to introspect them (dig into them to learn about the code, docstrings, etc) and doing anything like this upon import will break these tools.

One specific example is when you run help() in a Python REPL and type modules. This will import- and effectively run- every installed module.

So- I can't merge this. But it's also a bit of a chicken and egg problem. I can't check board type and call analog._add(four=AnalogInput(3, 3.3, None)) in setup() because setup() is called when you call automationhat.analog.four. IE: This analog channel wouldn't exist until you'd either explicitly called automationhat.setup() or used another channel or another input/output.

What probably needs to happen is for the warning to be raised (you can just ignore warnings, they aren't the same as a thrown exception) and for the analog channel to return some constant value, or None.

[Gadgetoid]

I think:

    def read(self):
        """Return the read voltage of the analog input"""
        if self.name == "four" and is_automation_phat():
            warnings.warn("Analog Four is not supported on Automation pHAT")

        self._update()
        return round(self.value * self.max_voltage, 2)

Probably needs to change to:

    def read(self):
        """Return the read voltage of the analog input"""
        if self.name == "four" and is_automation_phat():
            return 0  # Return a constant value, since ADC channel is floating

        self._update()
        return round(self.value * self.max_voltage, 2)

I think it's worth removing the warning altogether since (as you pointed out) it will show up in situations where you haven't explicitly asked for channel four, and it's more complicated to work around this than simply omit it.

[StefanZ8n]

I stopped working at my previous employer in Feb 2019 and didn't get mails about the updates, sorry guys (need to change my settings here). I understand the concerns and I think Gadgetoid's comment is a good alternative. I'll close this PR now. Thanks!