CroatianMeteorNetwork / RMS

RPi Meteor Station
https://globalmeteornetwork.org/
GNU General Public License v3.0
176 stars 49 forks source link

Fix running RMSExternalScript manually on machines with multiple stations #143

Open adalava opened 1 year ago

adalava commented 1 year ago

Adds --config/-c parameter that can be used to provide a different RMS configuration file instead of using the default one. This is useful when there are multiple stations running on the same machine.

Also fixes an error handling when the provided config file is not found.

Reported by: William Schauff and Washington Oliveira

adalava commented 1 year ago

@dvida , I'm not sure about the default behavior, I would like to help user avoid mistakes when running it manually. When a config file is not specified, I would like to suggest one of the solutions bellow (or one you and other may come up with): a) always load the .config file from Captured dir instead of the one in source/RMS/.config; b) when user doesn't specify a config file, check if the file in Capture dir is exactly the same as the one in source/RMS/.config before loading it. If not, interrupt the script and warn the user that there's a mismatch and suggest force a config file with --config/-c;

I'd vote option b :)

dvida commented 1 year ago

I agree with Mark's comments, the default behavior should be preserved. I.e. if a config file is not specified, use the one in ~/source/RMS.

adalava commented 1 year ago

There's no intention of changing the default behavior on this change: when --config parameter is not used (default case), a 'None' object is passed to loadConfigFromDirectory(), then ConfigReader will use the already existing logic[1] to load the default ".config" file (This is similar to the original config file load code in RMSExternalScript)

The try/except block is needed because when a config file is provided and the file is not found it raises a FileNotFoundError exception that need to be handled, otherwise user will get the unhandled exception stack trace instead of a friendly error message. (I added it to the FileNotFound exception to keep consistent with the code just above that raises the exception when default file is not found)

The original idea was to always load the config file from "data" directory following the '.' behavior in [2]. I think it would be ideal but it would change default behavior for sure.

[1] https://github.com/CroatianMeteorNetwork/RMS/blob/c3b5b64139fb22b10c81a6ce087a98418df7e01a/RMS/ConfigReader.py#L198 (lines 159 and 198) [2] https://github.com/CroatianMeteorNetwork/RMS/blob/c3b5b64139fb22b10c81a6ce087a98418df7e01a/RMS/ConfigReader.py#L165