Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add new python screen for ic2 #116

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

add new python screen for ic2 #116

wants to merge 2 commits into from

Conversation

mrjones-plip
Copy link
Owner

@mrjones-plip mrjones-plip commented Aug 21, 2024

This PR enables a new style of I2C script which:

  • largely copies existing script, so has nothing too new, but also has poor code re-use :(
  • has just 3 lines instead of 4. the 3 are much more legible
  • also reads temps from BME280 sensor and then does a POST to YANPIWs server
  • has some hard coded items, but they're called out with a todo flag. we'll live today to fight tomorrow.

@mrjones-plip mrjones-plip requested a review from NegativeK August 24, 2024 19:34
Comment on lines +18 to +31
parser = argparse.ArgumentParser()

# Rev 2 Pi, Pi 2 & Pi 3 uses bus 1
# Rev 1 Pi uses bus 0
# Orange Pi Zero uses bus 0 for pins 1-5 (other pins for bus 1 & 2)
parser.add_argument('--bus', '-b', default=0, type=int, help='Bus Number, defaults to 0')

# IP address of your YANPIWS device you want to show data from
parser.add_argument('--remote_ip', '-ip', default='192.168.68.105', type=str, help='Temp sensor ID, defaults to 0x76')

# ID from your YANPIWS config.csv of temp 1
parser.add_argument('--temp_id1', '-id1', default='143', type=int, help='remote temp ID #1, defaults to 143')

args = parser.parse_args()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ArgumentParser results don't need to be globals; the processing code can be in main or a called function.

Comment on lines +33 to +37
# Build URL and set some vars
yanpiws_ip = args.remote_ip
yanpiws_temp_1 = args.temp_id1
yanpiws_ajax_url = 'http://' + str(yanpiws_ip) + '/ajax.php?content='
last_seen_temp= 0.0
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto here -- these can be returned from a function, potentially as part of a class or a dataclass (see https://docs.python.org/3/library/dataclasses.html ).

If a global constant is absolutely needed, Python convention is to write it in all caps to remind users to not much with its state -- but it provides no actual write protection. Example: MILES_TO_KM_RATIO = 1.61

Comment on lines +39 to +43
# set up syslog logging
my_logger = logging.getLogger('MyLogger')
my_logger.setLevel(logging.DEBUG)
handler = logging.handlers.SysLogHandler(address='/dev/log')
my_logger.addHandler(handler)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto here, though less critical since the rest of the code isn't modifying the logger's state. Would recommend a configure_logging (or similar) function called by main.

Since a global is still (awkwardly) needed, having my_logger = logging.getLogger(__name__) right below the imports is common. __name__ means logging configuration and further details are automatically broken down by module, aka filename.

Python's logging documentation is a bit of a mess, but it's still useful.

Comment on lines +45 to +48
# BME280 sensor address (default address), Initialize I2C bus and calibration
address = 0x76 # can also be 0x77 - check i2cdetect -y 1 or i2cdetect -y 0
bus = smbus2.SMBus(1) # can also be 0, depends on where you found device on per i2cdetect
calibration_params = bme280.load_calibration_params(bus, address)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto here with globals.

address might be a good candidate for a global constant.

bus = smbus2.SMBus(1) # can also be 0, depends on where you found device on per i2cdetect
calibration_params = bme280.load_calibration_params(bus, address)

def post_to_yanpiws():
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: Architecturally, this might be a good candidate for a class (or a closure if you want to be functional.) A number of variables don't change after initialization, and last_seen_temp could be an attribute of the class instead of a global.

return raw_html


def get_remote_humid_and_temp(id):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't cause a bug here, but id is a reserved word.

humid_and_temp1 = get_remote_humid_and_temp(yanpiws_temp_1)
forecast = get_remote_forecast()
first_line = get_remote_sun()
except:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A catch all except gives me the heeby jeebies, but it might not be worth fixing..



def full_stack():
import traceback, sys
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recommend hoisting import to the top of the file.

return stackstr


def main(device):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused parameter -- show_info is using the global device.

if __name__ == "__main__":
try:
my_logger.debug('Weathercaster: simple Starting ')
serial = i2c(port=args.bus, address=0x3C)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0x3c might be a good candidate for a constant at the top of the file versus a magic number.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants