Confusion about 'sensor'

[Cabalist]

Hello again!

I have a question about one of the most used variables in mycodo.py. sensor is used all over the place and I believe it is an int and the address of a sensor. But I am not sure.

I see that when it is used in logging it tends to have 1 added to it. This makes me think that internally the sensors are numbered 0-7 but in the log you refer to them as 1-8.

Could you clarify what this variable is used for in mycodo.py?

[kizniche]

This is the number of sensors for each class of sensors (temperature, temperature/humidity, co2, pressure). It's used mainly to identify settings by being used as an array index, such as sensor_t_activated[sensor]. Yes, the 1 is added for display purposes to the user (logs, web interface).

[kizniche]

You'll probably notice the daemon runs with many global variables. I've been trying to be better at moving away from globals. I've found ways to make many of them local, but there's probably a lot more that can be done. I started creating the daemon with all globals, so it's a slow process to fix some of my initial mistakes.

[Cabalist]

Would renaming sensor to sensor_id make sense to you? That way it isn't confused with a sensor object or anything besides an index.

Yeah there are a LOT of globals. I've been trying to suss out whether they were there for GPIO pins or if it was a coding style. Sounds like it is a coding style that you'd like to move away from. Your code works and people seem to enjoy it. I hope you aren't worried about the style mistakes. 😄

[kizniche]

I agree, sensor_id makes more sense. 788 matches for the word 'sensor' 😦

I have a lot of bad coding style habits. I don't mind criticism, as long as I can learn something. I'm sure there are a lot of armature mistakes throughout all my code. I used to care only about functionality, but I also wanted to share my code. So I realized there needs to be a balance between functionality and readability.

[Cabalist]

I'll make a pull request for this. One moment.

Just 709 that are the full word and not in comments or literals.

[Cabalist]

Hmm... be careful not to do a blind find/replace.

This looks like a bug:

   def exposed_ReadCO2Sensor(self, pin, sensor):
        logging.info("[Client command] Read CO2 Sensor %s from GPIO pin %s", sensor, pin)
        if sensor == 'K30':
            read_co2_sensor(sensor - 1)
            return sensor_co2_read_co2
        else:
            return 'Invalid Sensor Name'

It checks wether sensor is a string and then tries to do math on it.

[kizniche]

Oh, I see it now. That is rather odd.

[kizniche]

Should be:

    def exposed_ReadCO2Sensor(self, pin, sensor):
        logging.info("[Client command] Read CO2 Sensor %s from GPIO pin %s", sensor, pin)
        read_co2_sensor(sensor - 1)
        return sensor_co2_read_co2

[kizniche]

...Another bad use of a global.

[kizniche]

Actually, this is what it should be:

    def exposed_ReadCO2Sensor(self, device, sensor):
        logging.info("[Client command] Read %s CO2 Sensor %s", device, sensor)
        if device == 'K30':
            return read_co2_sensor(sensor - 1)
        else:
            return 'Invalid Sensor Name'

[Cabalist]

Don't you mean.... sensor_id? haha

[kizniche]

Haha. Yeah. I actually did use sensor_id in the commit. There was a lot to fix in that small function.

[Cabalist]

So looking at the fixed exposed_ReadCO2Sensor I notice that instead of doing +1 in the log string you are -1 when passing it to the read_co2_sensor function. This is worth raising because some of your code does this the other way around.

Is there a reason to treat the sensor_ids differently here?

[kizniche]

When going from the stored value, the index starts at 0, so 1 must be added when presented to the user, but when receiving as a user input, 1 must be subtracted to reach the appropriate value that is stored in the database.

[kizniche]

The exposed functions are used to execute code from the mycodo-client.py script. I suppose we could have the client script subtract 1 before calling exposed_ReadCO2Sensor

[Cabalist]

I think that would be right. We want to make sure that abstraction layers don't mix. The 'core' should act consistently and if you make a user layer you talk to the core using that language. The DISPLAY may show other stuff. If the relays were distinguished by color (red relay, blue relay, etc.) we would still have use the numeric address internally but it would be up to the UI to convert that into red/blue/etc so that the human would know which one we were referring to.

It isn't wasteful to have something like this:

def exposed_ReadCO2Sensor(self, device, sensor):
    """
    User facing function takes sensor_id as numbered on webpage.
    """
    return readco2sensor(self, device, sensor - 1)

def readco2sensor(self,device, raw_sensor_id):
    """
    Internal implementation of readco2sensor.  May change over time or as sensors are added.  Consistent internal notation 
    """

    logging.info("[Client command] Read %s CO2 Sensor %s", device, raw_sensor_id + 1)
    if device == 'K30':
        return read_co2_sensor(raw_sensor_id)
    else:
        return 'Invalid Sensor Name'

[kizniche]

Got all sensor functions created as per that template, but getting this error when I test one:

File "/var/www/mycodo/cgi-bin/mycodo.py", line 364, in exposed_ReadHTSensor
    return readhtsensor(self, device, sensor_id - 1)
NameError: global name 'readhtsensor' is not defined

[kizniche]

Ah, think I need to call self.readhtsensor()

[Cabalist]

yep that would do it. :)

[kizniche]

Success!

kiz@tango:www/mycodo$ ./cgi-bin/mycodo-client.py --sensorht DHT22 1                          11/25/15 12:54 AM
2015 11 25 00 55 00 [Remote command] Read DHT22 HT sensor 1
2015 11 25 00 55 02 [Remote Command] Daemon Returned: Temperature: 25.9°C Humidity: 36.6%

[kizniche]

All finished. T, HT, CO2, and Press sensor querying from mycodo-client now working with the new functions.

[kizniche]

I'm going to have to start getting to bed earlier. I live on the east coast, so I'm a few hours ahead of you. Luckily it's a slow week and I can sleep in until noon, and I've thoroughly enjoyed the late-night programming sessions, but our schedules don't quite align. You may start to see me drop out around midnight EST, instead of 4am / 5am.

[Cabalist]

Good. Sleep is good. haha :)

On Wed, Nov 25, 2015 at 2:57 PM, Kyle Gabriel notifications@github.com
wrote:

I'm going to have to start getting to bed earlier. I live on the east
coast, so I'm a few hours ahead of you. Luckily it's a slow week and I can
sleep in until noon, and I've thoroughly enjoyed the late-night programming
sessions, but our schedules don't quite align. You may start to see me
drop out around midnight EST, instead of 4am / 5am.

—
Reply to this email directly or view it on GitHub
#57 (comment).

[kizniche]

Seems we've fully explored this topic. closing...