pico-clock-green-python icon indicating copy to clipboard operation
pico-clock-green-python copied to clipboard

display of day of the week is off

Open the-it opened this issue 4 years ago • 11 comments

the-it avatar Oct 13 '21 14:10 the-it

Functionally this is identical. only needlessly complicated. and not logically sound, as Sunday is the first day of the week, therefore 0-6 was correct. Title of the commit is also misleading. My previous commit correctly displayed the correct day of the week. unless you have changed the rtc to start with 1 based numbering instead of 0 based numbering.

cpyarger avatar Oct 14 '21 05:10 cpyarger

For me, it just shows the wrong day of the week (today on Thursday it shows Tuesday). Maybe we should implement the correct setting for day of the week somewhere in our code?

the-it avatar Oct 14 '21 08:10 the-it

@the-it I take it you have set the date correctly? The day of the week is derived from the date (Asking the stupid questions first)

malcolmholmes avatar Oct 14 '21 09:10 malcolmholmes

@the-it I take it you have set the date correctly? The day of the week is derived from the date (Asking the stupid questions first)

yes done that correctly. Even get a dump of the date in the console

the-it avatar Oct 14 '21 09:10 the-it

I set the day of the week manually when I first set the RTC. and once the DOW is set it is pulled directly from the RTC not calculated.

cpyarger avatar Oct 14 '21 13:10 cpyarger

I think we should set the DOW in the init routine of the RTC object. If code relies on a correct setting ... we should set it accordingly.

the-it avatar Oct 19 '21 11:10 the-it

Is the DOW just an incrementing number in the RTC? i.e. is it down to code to make sure that the DOW is correct in relation to the date? In which case, correcting the DOW from the date in code makes sense.

malcolmholmes avatar Oct 20 '21 08:10 malcolmholmes

I believe you only need to set the DOW when setting the time. and Yes its an incrementing 1:1 integer coming from the RTC

cpyarger avatar Oct 20 '21 09:10 cpyarger

@cpyarger so we need to check if the time setting code is correct regarding weekday. @the-it can you take a peek?

malcolmholmes avatar Oct 20 '21 10:10 malcolmholmes

Hi, I had the same problem. I think the cause is that the weekday number is shifted up and down in several places in the code.

On line 79 in ds3231_port.py it saves the weekday to the DS3231 RTC after adding 1.

When converting to localtime format (I'm not sure why that is necessary), ds3231_port.py on line 64 subtracts 1 from the weekday. (It also subtracts 1 on line 116 and line 128 when doing rtc_test().)

The list 'day_of_week' in display.py (line 184) is in the wrong order, with Sun as 0 and Mon as 1. I'd call that wrong because the clock shows the icons Monday-first, and all datetime functions in Python and MicroPython report the weekday between 0 and 6, where 0 is Monday.

So when cleaning that up, my clock shows the right icon for today's date. The RTC module, the system time (import time; time.localtime()) and the time the clock is working with are all in agreement now what the weekday is (I checked by printing to the shell).

I did have to reset weekday in the RTC manually, because --if I'm following the code correctly-- the RTC module will only be updated when it is not already set. So if you have a wrong weekday in there, it will only be corrected when the date changes I guess? Or when you take the battery out of the RTC module :-) The easier way is to run a script from Thonny to change the Pico's systems time (and maybe other settings). I will add that script and turn this whole comment into a PR later.

twisst avatar Aug 03 '22 13:08 twisst

Just to add some background, I think what may be the source of confusion is that there seems to have been a bug in MicroPython's port for Pico:

https://github.com/micropython/micropython/issues/7889 https://github.com/micropython/micropython/pull/8250/commits/5679fe6aee7811447ab9dfc9b4539526b23b1d84

That means that an offset was needed before, in the original software for this clock, but now that can be left out.

twisst avatar Aug 03 '22 15:08 twisst