search

twVolc / PyCamPermanent

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

Reset stray_pix values on `doas_worker.dir_load()` #130

Closed ubdbra001 closed 1 month ago

ubdbra001 commented 1 month ago

Fixes #118

See issue for more details.

twVolc commented 1 month ago

this looks good to me, but I'm just thinking that this won't fix every case - if we have the default spectrum loaded, but then move to acquiring data - which starts a watch directory by calling IFitWorker.start_watching(), if the spectra that now come in don't match the default spectrum (or whatever spectrum is currently loaded) I think we'll run into the same issue that we won't get things processed properly. I think if you add your new reset_stray_pix() method to IfitWorker.reset_self() too this will account for this use case (reset_self() is called within one of the methods in start_watching()). Does that make sense?

ubdbra001 commented 1 month ago

I see what you mean, I'll change that now.

Out of interest is the DoasWorker defunct for processing this kind of data (in your mind at least)? If not, then I think it would be useful to add a parent class for both IfitWorker and DoasWorker that implements their common features, just to make code maintenance easier. Not in this PR but for future work.

twVolc commented 1 month ago

Out of interest is the DoasWorker defunct for processing this kind of data (in your mind at least)?

At the moment it's completely defunct unfortunately - I don't think it would run at all if we tried to use it as i haven't kept up with updating for quite a log period of time. I've seen you're continuing to add to both, which is great as it would at some point be good if I can get this back to working properly as it can be nice to compare DOAS with iFit.

But yes your note on pulling out commonalities into a parent class definitely makes sense and would make keeping on top of the DOASWorker much easier. The only issue is overall this is probably quite an undertaking, so finding the time to do it isn't straightforward as I can't justify prioritising DOAS stuff.

ubdbra001 commented 1 month ago

The only issue is overall this is probably quite an undertaking, so finding the time to do it isn't straightforward as I can't justify prioritising DOAS stuff.

That's fair enough, but now I know it's something that would be useful it's maybe something that I can return to in future. I'll add an issue for it now. Plus, at least if you know that the core features of DoasWorker work in the same way as IfitWorker then it makes updating it a bit less of a headache.