Modularize sensor/device code about mycodo HOT 17 CLOSED

@Cabalist What are your thoughts on this?

from mycodo.

I've done something similar for MycodoLog.py, MycodoGraph.py, and MycodoPID.py, but I'm not sure if this is the best approach. Additionally, these modules are always loaded by the daemon. Being able to selectively load modules is key to the proposed functionality.

from mycodo.

Some questions:

• Should a SQL query be performed each time the sensor_read() function is called to pull settings for the particular sensor being read?
• Alternatively, these settings can be passed as parameters to the function so the SQL query only needs to be performed once in the main loop.

from mycodo.

More questions:

• Currently there are global variables that are used to terminate the server and pause the daemon (client_que == 'TerminateServer' and pause_daemon). Could this system still work if, for instance, the functions read_ht(), read_ht_sensor(), and verify_ht_sensor() were put into their own module that is loaded with import?
• If not, or if it shouldn't be done, what is the best approach to terminating or pausing the processes in these functions if the daemon needs to terminate right away or pause?

from mycodo.

It seems as though the only functions that should be modularized (or broken down to smaller modules from each) are read_t(), read_ht(), read_co2(), and read_press(). The reference to breaking down to modules from the function refers, for instance, to read_ht(), for which there are multiple different sensors (the other classes of sensors only have one sensor each) and would potentially need a module for the DHT and a module for the AM2315.

from mycodo.

These are all good questions and a lot of what I'd like to help contribute. I don't have the answer to most of them at the moment.

I think the way to move forward is:

1. Rename common code snippets to illustrative variable names. This will help us identify repeated code.
2. Remove globals and add more functions to reduce repeated code.
3. Add Classes for certain data types. I believe that we have probably 2 or 3 classes that will eventually form from the way that the data is accessed.
4. Write tests. So we don't break anything along the way.
5. Possibly move to SQLAlchemy to handle DB stuff. Could simplify some things.
6. Decouple threading from functions and put it in a different module.

For example:
read_t(), read_ht(), and read_co2() are all (sorta) the same function. They read from a sensor and return a value. I think we can combine them and then have some other functions that makes sense of the value they return. This will reduce code complexity by a great deal.

mycodograph.py is... unique. It probably would have been a better place for me to start since it is self contained. It is great that it is it's own module because it has a well defined start and end. We know exactly what It should do. No globals! However I think it is the largest single function I've seen. It definitely has the most arguments I've ever seen! haha. Breaking that up into pieces would be the next exercise. I bet that is 20-30 functions that all construct a small part of the whole and then you use another function to assemble those parts and return it to the caller.

I'm still trying to get all the code in my head. Does that all sound in line with what you are envisioning?

from mycodo.

That all sounds great. You made me laugh with

mycodograph.py is... unique.... No globals! I think it is the largest single function I've seen. It definitely has the most arguments I've ever seen!

I'm both honored and appalled

Seriously, though. mycodograph.py was the last big thing I created, to replace (but now complements) the static graph (server-side generated), as a client-side generated graph (with more features). It was my first attempt at moving completely away from using any globals. I just wanted to get it working at all costs, then go back and make it more efficient and readable. Slowly but surely I'll learn, and when I create things in the future, they'll be (more) logical from the start!

from mycodo.

Also, If you want to have a Mycodo login on one of my Pis (w hum/temp, co2, and pressure sensors), let me know how I can send you the connection/auth details.

Edit: Here's an easy way to contact me

from mycodo.

What's your background with programming/Python, if you don't mind me asking?

from mycodo.

Email sent!

Eh. I own a company in Portland. We do python development with Point of Sale and Retail (Lightspeed POS and SAP) and Web Development (Flask mostly). I need some side projects that don't make me go crazy and I've been watching this one for awhile. It's fun. One of our programmers is a Biology Major at PSU (@etiology) and that makes it easier to work on this.

I've been programming for about 3-4 years. I read a lot of books and really like the Pythonic way of doing things. I didn't go to school for it but learned it out of necessity. Before this I worked as a screwdriver jockey at a bunch of random computer shops all over the US

Book Recommendation: You obviously know how to program so I'd recommend this book. It has a lot of very small tips that demonstrate good code usage. I think it helps writing better code in general. :)

Honestly I picked this project because it seems like you have been consistently working on it for awhile and didn't just get sick of it. It means that I am not screaming into the void when I make pull requests. :)

from mycodo.

Thanks! And I'm glad to see you like this project. Can you give an example of this so I might begin working on these?

Rename common code snippets to illustrative variable names

from mycodo.

We already did it once with the relay active code. I'm looking to attack some of the other conditionals to see if we can find more examples. I'll probably send you some questions about what different pin combinations mean.

from mycodo.

A good example:

if ((pid_ht_temp_set_dir[sensor_id] == 0 and pid_ht_temp_relay_high[sensor_id] != 0 and pid_ht_temp_relay_low[sensor_id] != 0) or (pid_ht_temp_set_dir[sensor_id] == -1 and pid_ht_temp_relay_high[sensor_id] != 0) or (pid_ht_temp_set_dir[sensor_id] == 1 and pid_ht_temp_relay_low[sensor_id] != 0)) and pid_ht_temp_or[sensor_id] == 0 and pid_ht_temp_down == 0 and sensor_ht_activated[sensor_id] == 1:

from mycodo.

That's a fun one!

from mycodo.

set_dir defines if the PID is going to regulate up (1), down (-1), or up and down (0).
relay_high is the relay used to lower a condition (e.g. cooling device to lower temperature).
relay_low is the relay used to raise a condition.
This is making sure that whatever direction the PID regulation is going to go, there is a relay or relays set (not 0, set to the relay number) to do so.

temp_or is the override. When set to 1, the PID is not active.
temp_down is used to shut down the PID thread (when set to 1).
activated determines if the sensor is actively logging (1) or not (0).

from mycodo.

Real quick PID means Pin ID or Process ID?

from mycodo.

Proportional Integral Derivative, as in PID controller, which is used to determine the amount of time the relay turns on for, based on the input condition (e.g. temperature), set point (i.e. desired temperature), and Kp, Ki, and Kd gains.

from mycodo.