PR/Branch for "new" plugin?

[rccipriani]

I was having trouble with wttr.in being down and made a copy of the weather plugin using a site called OpenMeteo. If I were to submit a PR, would that go to a new branch or an existing one?

[BrookeDot]

How much changes when you switch to OpenMeteo? If it's not a complete re-write/refactor of the weather plugin my thought would be to add a constant to the constants.h file something like WEATHER_PROVIDER that accepts wttr or openmeteo.

Then in the weather plugin itself do a conditional that checks which service to use.

To answer your original question regarding creating a PR, generally the way that would work is a "feature branch" would be created, something like feature/add-openmeteo-weather created in your fork then on this repo you would create a PR to merge that PR into main assuming no conflicts.

In other words:

• Fork the main branch into a repo you have write access to
• Create your feature branch, and make the changes.
• Once ready, request a PR from your local feature branch to the main branch here.

[rccipriani]

Not a whole lot. It needs lat and long in separate variables. Obviously the URL changes. The other thing is the weather codes are different from wttr.

[BrookeDot]

That makes sense, sounds like the conditional approach may be a good one.

Not a whole lot. It needs lat and long in separate variables.

A few options here IMO, either have them be set in the constants file as separate variables (easiest and likely preferred), or do a split on , on WEATHER_LOCATION. For example, I use cords for my location: 1234,5678 but from a user perspective a lot more to go wrong with option 2, even if it's fewer settings.

Open to other feedback, but my personal vote would be to just use a conditional inside the current plugin. Here is some pseudo code, you can test, and wrap the same if statement around the actual output as well.

#ifdef USE_OPENMETEO && (defined (WEATHER_LAT) && defined (WEATHER_LONG) )
    String weatherApiString = "https://api.open-meteo.com/v1/forecast?latitude=" + WEATHER_LAT + 'longitude=' + WEATHER_LONG + '&hourly=temperature_2m&current=temperature_2m';
#else
    String weatherApiString = "https://wttr.in/" + String(WEATHER_LOCATION) + "?format=j2&lang=en";
#endif

    

[c64emulator]

Not a whole lot. It needs lat and long in separate variables.

A few options here IMO, either have them be set in the constants file as separate variables (easiest and likely preferred), or do a split on , on WEATHER_LOCATION. For example, I use cords for my location: 1234,5678 but from a user perspective a lot more to go wrong with option 2, even if it's fewer settings.

I would prefer not a hard coded WEATHER_LOCATION, would instead appreciate a configuration front end to set the location and some other parameters (e.g. an API key)