rgleason / weatherfax_pi

Weatherfax Plugin for Opencpn
GNU General Public License v3.0
5 stars 9 forks source link

WeatherFaxInternetRetrieval.xml size #59

Closed ThomTrab closed 3 years ago

ThomTrab commented 3 years ago

@rgleason, before starting to work on a PR, I would like to discuss the matter with you.

For me WeatherFaxInternetRetrieval.xmlreach an unmanageable size. There is today ~2500 lines in this file, which make it very difficult to update manually. I would like to propose to split the file. How is not clear to me now.

I see two ways, either by area (EU, US, Asia, Africa,....) or by server (LaMMA, NOAA, Pwx, ...).

Doing it by server will require to add automatic listing (list data folder content using regexp) and then iterate on this list. I don't know how to do this in C++. The main advantages of this solution is that we could add any other file in the future as we could see fit.

Doing it by area will require fewer file and these could be hardcoded in InternetRetrievalDialog.cpp. I think I could manage to do it on my own. The downside of this solution is that at some point, I think we will reach the same issue as today.

Do you have an opinion on the matter ?

rgleason commented 3 years ago

I think either way would work. I like the simplicity of breaking it up by areas or regions. But I think Sean might be able to help with the second proposal which makes a lot of sense. Also Sean should be involved and will have some thoughts. I will alert him to this post. @seandepagnier

BTW I need to get your recent pr into master branch. Have now merged and will push to Sean too. Emailed Sean too.

rgleason commented 3 years ago

I think separate files for each URL would be good, but I am also thinking about the Coordinatessets.xml too. Is there a way to keep that manageable too? Because coords are user writable they need to be accessed from a user writable directory. Currently weatherfaxinternetretrieval.xml is in the protected data directory.

seandepagnier commented 3 years ago

I honestly don't understand what difference the file size makes, but I am also not sure xml is the best, and if re-organizing it helps.

I am not opposed to any changes unless they break functionality.

ThomTrab commented 3 years ago

I think separate files for each URL would be good

I did this for the most part. Server with only few lines have been kept together either by geographical area or in the misc file. PR coming your way.

I am also thinking about the Coordinatessets.xml too. Is there a way to keep that manageable too?

I though of it but this is out of my league as it need to specify the according file when writing the file to disc. I don't know C++ enough to do this.

I honestly don't understand what difference the file size makes

To update the file, I basically scroll through it to find the place I would like to modify. 2500+ lines is a lot to scroll through :wink:

I am also not sure xml is the best, and if re-organizing it helps

As for the CoordinateSets file, moving away from XML is out of my C++ league :wink:. So for now, I can only propose small modification to a working tool, not rewrite part of it.

rgleason commented 3 years ago

ThomTraub, I've merged the changes and tested them and they seem to work fine.

I wonder if the coordinateSets.xml could be separated into separate files (with parallel naming)? but since these files are copied out to the user writable data directory, there would have to be some additional work done on the initialization routines.

rgleason commented 3 years ago

I am going to close this now. Thanks very much. I think it will be easier to maintain.