Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Weather: temp won't display in Fahrenheit #1783

Closed
1 task done
5iver opened this issue Jun 26, 2023 · 4 comments · Fixed by #1805
Closed
1 task done

Weather: temp won't display in Fahrenheit #1783

5iver opened this issue Jun 26, 2023 · 4 comments · Fixed by #1805
Labels
bug Something isn't working

Comments

@5iver
Copy link

5iver commented Jun 26, 2023

Verification

  • I searched for similar bug reports and found none was relevant.

What happened?

Weather only reports in Celsius

What should happen instead?

When specified, weather should report in Fahrenheit

Reproduction steps

I am using Gadgetbridge (0.74.0) with the QuickWeather (2.3.2) app on Android. GadgetBridge is configured for Imperial and QuickWeather is configured for Fahrenheit. My family all have PineTimes running InfiniTime 1.13.0 and they are displaying the temperature in Celsius.

More details?

I do not see any relevant issues reported for either of their apps. A quick look through their repositories did not lead me to believe the issue was elsewhere, especially because PineTime recently implemented weather functionality. Though, I could absolutely be incorrect and apologize in advance if that is the case.

Has anyone seen Fahrenheit temperatures using the new weather feature?

Version

1.13.0

Companion app

Gadgetbridge 0.74.0

@5iver 5iver added the bug Something isn't working label Jun 26, 2023
@kieranc
Copy link
Contributor

kieranc commented Jun 28, 2023

This is correct, there's no imperial support yet, I believe it should be fairly easy to implement so I'll leave some notes here in case anyone gets to it before me.

This is how it's implemented for the Pebble. It checks if GB is set to use imperial units and if so, recalculates the temperature sent to the watch. The watch deliberately doesn't include units after the degrees symbol so it should display fahrenheit just fine, at least below 99 degrees, after that there probably isn't space to show 3 digits. So don't go anywhere too hot.

This is the relevant section for Pinetime.

All this is wrong, I tried and it doesn't work. A toggle on the watch for metric/imperial is looking like the way to go.

@Brod8362
Copy link

@kieranc and I talked a bit on discord and I want to summarize our conversation here:

  • It seems like it could be possible to approach this from either the companion app side, or performing the conversion on the watch
  • By performing it on the watch, any companion app that publishes weather data should be supported
  • It should be easy for the user to switch their preferred unit (e.g no recompiling)
  • For now, it seems like just having another setting is ideal.

I'm still familiarizing myself with the WeatherService and how it is used, but it should be easy enough to change WeatherService::GetCurrentTemperature to just convert to fahrenheit within that function (when set). The watchfaces wouldn't have to do anything special to support this (aside from being prepared to render 3 digits).

Currently, though, I'm looking into simulating the weather controller service from within InfiniSim, to make development for this easier.

@FintasticMan
Copy link
Member

FintasticMan commented Jun 28, 2023

I would personally prefer the conversion to be done on the watch, rather than on the companion app. I would say we should then add a GetFahrenheit() function to the WeatherData::Temperature class (a reference to which is returned by the GetCurrentTemperature() function).

@Brod8362
Copy link

That seems like a pretty elegant solution. I'm mostly working on trying to see how feasible it will be to add weather integration to InfiniSim.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants