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

Exception handling for weather fetching failures #10

Closed
surak opened this issue Oct 10, 2018 · 13 comments
Closed

Exception handling for weather fetching failures #10

surak opened this issue Oct 10, 2018 · 13 comments
Labels
enhancement New feature or request outdated This issue is from a previous release and is most likey not relevant. (Archived)

Comments

@surak
Copy link
Contributor

surak commented Oct 10, 2018

When the weather service fails, the code throws an exception and dies.

As there is no watchdog to relaunch the process (should I write one?), a better handling of the possible exceptions (fetching ics file, fetching weather, drawing the screen) should have try/catch statements.

I am creating issues separately, should I just use the suggestion/improvements? Or just pull requests (complicated, since the api key, city name and ics url are on the main code file). Which leads me to the next issue...

@aceisace
Copy link
Member

@surak
Currently, there is nothing to restart the script or a section of the script if there is a network or server problem except for rebooting.

I'd recommend the latter (using try/catch statements) in the script as it's better suited for debugging than a watchdog that keeps relaunching the script. Unfortunately, I'm not too good at exceptions handling so I'll have to ask you for help on that.

I am creating issues separately, should I just use the suggestion/improvements? Or just pull requests (complicated, since the api key, city name and ics url are on the main code file). Which leads me to the next issue...

It's probably better to have seperate issues for different topics than mixing them all up in the suggestions/improvement issue. Pull requests are fine too and you've made a good point that the config file should be kept seperately.

@aceisace aceisace added the enhancement New feature or request label Oct 22, 2018
@surak
Copy link
Contributor Author

surak commented Oct 23, 2018

I thought about something that would make it CLEAR on the screen that it's not being updated. Like a warning in some corner, or just a huge X crossing the whole thing, making us both annoyed and wishing to fix it. What do you think?

@aceisace
Copy link
Member

Yes, that‘s a good idea. As the Openweathermap server is available for ~95% a day for free users, it‘s possible there was a server error than a network problem. In that case, retrying to access the API will fix it.

I’d prefer the second option, a small warning somwhere on the display.

What do you say about a crossed out Wifi icon in the place of the weather icon when the server returned an error?

@surak
Copy link
Contributor Author

surak commented Oct 24, 2018

I’d prefer the second option, a small warning somwhere on the display.
What do you say about a crossed out Wifi icon in the place of the weather icon when the server returned an error?

That's a good one. Together with a warning on the console, like you have on the pull request.

@aceisace
Copy link
Member

@surak
Would the icon below be suitable for showing a network/server problem? It has the same dimensions as the weather icons (60x60px) and can be used in place of the weather icon.
no-wifi

I'll also add a warning on the console to show that there is an error.

@surak
Copy link
Contributor Author

surak commented Oct 24, 2018

It is somehow resized here (appears as a 95x95 jpeg), but I'm thinking if this isn't somewhat misleading: That is clearly the wifi, which might lead the user to think that there is, in fact, a problem with the wifi, not with the OpenWeather. How about a "broken" cloud?

Something like this (I just googled, and I can't upload svg here):

@aceisace
Copy link
Member

@surak
You have a point actually. Since it's not clear if the issue lies with openweathermap or the network itself, it's probably not a good idea to use my suggested icon. I'll create an icon similar to the third icon (a cloud with question mark) and upload it again here.

Also, just modified your comment to make the pictures a bit smaller

@aceisace
Copy link
Member

@surak
OK, this is the icon I just made, using the third icon as the template:
cloud-no-response-alt
And as an alternative:
cloud-no-response
I think the latter is slightly better to read. What do you think?

@surak
Copy link
Contributor Author

surak commented Oct 24, 2018

I'm with you on the latter!

@aceisace
Copy link
Member

aceisace commented Oct 25, 2018

I’d prefer the second option, a small warning somwhere on the display.
What do you say about a crossed out Wifi icon in the place of the weather icon when the server returned an error?

That's a good one. Together with a warning on the console, like you have on the pull request.

I've tested out a few improvements to the main file:

  • Created a seperate file 'icon_positions_locations.py' for positions of icons and their locations.
  • Added feature to show icon when Openweathermap API returns an Error
  • The main file will now print more information on the console (see below)
  • Added option to show upcoming events with name and date on the console
  • Added more explanations in the main file on what the functions and sections do.

Here is the link for the improved stable.py:
improved main script

The corresponding additional files (icon: 'cloud-no-response' and file: 'icons_positions_locations.py') have been added to the repo's 'Calendar' directory.

Could you please also test the improved stable.py file? If you're content with that, I'll replace the current stable.py file with the improved one.

@aceisace
Copy link
Member

aceisace commented Nov 4, 2018

@surak
OK, I've tested out the new stable.py file for over a week now and so far, without any issues. After a few more minor improvements, I'll release a new version (1.4), sometime this month.

By the way, what do you think of the current output of the main file (see picture above)?

@surak
Copy link
Contributor Author

surak commented Nov 4, 2018

Hi! Sorry for my lack of communication. I’m preparing for a conference right now.

My rig is working nonstop since the last time I sent you a picture (It updates itself every five minutes). I’ve been thinking a lot about the upper part. Turns out that we look at it to check the temperature. Always. And it’s too small.

So I’ll play around with some designs that give some proheminence to the temperature and weather for the rest of the day. Which is what matters. The weather of the day and tomorrow. The rest can be small.

Turns out that time is useless, when you have so many clocks around! And even more when you burn the e-ink with it.

About the main file, I was doing something similar already. I think that everything of interest should go to the console as well. It’s “free” anyway :)

@aceisace
Copy link
Member

aceisace commented Nov 5, 2018

@surak
Hi, no problem.

It's good to hear that your rig is working fine. It also means that this issue is resolved so I'll be closing this. If you feel a need to re-open this issue, please do so.

If the space above the Calendar section is too small, you can always create a new page. For example, after touching a capactive sensor, the programm will switch to the other page. Another idea would be to add an OLED display via the GPIOs and fetch real-time data from a sensor.

In the former case, I can help you by publishing an example 'template' file which shows how to create a new page, add functions etc.

Although the idea of showing the time on the E-Paper display is a good one, for this specific E-Paper display, it's not a good idea. The reason is that, as you mentioned, ghosting of the display which requires more 'calibrations'.

If you have any more suggestions on the main file (like a console output I might have forgotten), please use the improvement ideas issue for that purpose.

@aceisace aceisace closed this as completed Nov 5, 2018
@aceisace aceisace added the outdated This issue is from a previous release and is most likey not relevant. (Archived) label Apr 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request outdated This issue is from a previous release and is most likey not relevant. (Archived)
Projects
None yet
Development

No branches or pull requests

2 participants