twVolc / PyCamPermanent

Permanent PiCam (SO2) installation project software
GNU General Public License v3.0
2 stars 2 forks source link

change when `retrieve_schedule_files()` is called #182

Open ubdbra001 opened 2 months ago

ubdbra001 commented 2 months ago

https://github.com/twVolc/PyCamPermanent/blob/8abcc5b0a0c0db5e40d8f0e2b30025a10af8b903/pycam/networking/FTP.py#L360-L375

I'm looking at this section of code, and the self.retrieve_schedule_files() feels a bit out of place (it'll be called every time test_connection() is run, which feels a bit redundant).

I think your aim was to have it get config files from the Pi when an initial successful connection is made (correct me if that's wrong), so maybe it would be worth having it run when the class is created and when the connection is updated, if a successful connection is opened, e.g.:

# at the end of __init__():
if len(self.host_ip) > 0:
    self.open_connection(self.host_ip, self.user, self.pwd) and self.retrieve_schedule_files()

# the 'and' works as a short circuit, so the second function is only run if the 1st function returns True

# at the end of update_connection():
self.test_connection() and self.retrieve_schedule_files()

What do you think @twVolc ?

twVolc commented 1 month ago

I think you're right - separating out these functions seems to make sense. I don't know for certain why I put that there, but my guess is that I sometimes experienced issues with the connection even after the initial setup, so adding the extra test of actually retrieving some files gives a robust way of testing that the connection is deifnitely open and working. In reality having the retrieve run afterwards is absolutely fine.

I'd also guess that those TODOS saying it hasn't been tested are now redundant, but can't say for certain. I guess the fact that the software works suggests those parts do work...