Codecov Report

Attention: Patch coverage is 0% with 8 lines in your changes missing coverage. Please review.

Project coverage is 98.53%. Comparing base (406bccc) to head (9afbeb1).
Report is 39 commits behind head on master.

@@            Coverage Diff             @@
##           master   #16357      +/-   ##
==========================================
- Coverage   98.57%   98.53%   -0.04%     
==========================================
  Files         164      164              
  Lines       21349    21357       +8     
==========================================
  Hits        21045    21045              
- Misses        304      312       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Update: I also added the possibility to initialize the RTC only with the date. According to the docs the time related parameters are optional.

(year, month, day[, hour[, minute[, second[, microsecond[, tzinfo]]]]]

New test:

from time import sleep
from machine import RTC

rtc = RTC()

year = 2024
month = 12
day = 26
weekday = 3
hour = 6
minute = 30
second = 20
microsecond = 0
tzinfo = 0

def test(source, assert_time = True, should_fail=False):
    print(f"\nTesting {source}")
    # Read time
    t = rtc.datetime()
    print(t)
    try:
        assert t[0] == 2024, f"Year should be 2024 but was {t[0]}"
        assert t[1] == 12, f"Month should be 12 but was {t[1]}"
        assert t[2] == 26, f"Day should be 26 but was {t[2]}"
        assert t[3] == 3, f"Weekday should be 3 but was {t[3]}"
        if assert_time:
            assert t[4] == 6, f"Hour should be 6 but was {t[4]}"
            assert t[5] == 30, f"Minute should be 30 but was {t[5]}"
            assert t[6] == 20, f"Second should be 20 but was {t[6]}"
        if should_fail:
            print("❌ Test should have failed but didn't")
        else:
            print("✅ Test passed")
    except (AssertionError, ValueError) as e:
        if should_fail:
            print(f"✅ Test failed as expected: {e}")
        else:
            print("❌ Test passed but should have failed")

# Initialise the RTC. Datetime is a tuple of the form:
# (year, month, day[, hour[, minute[, second[, microsecond[, tzinfo]]]]]
rtc.init((year, month, day, hour, minute, second, microsecond, tzinfo))
test("init in order")

rtc.init((year, month, day, tzinfo, hour, minute, second, microsecond))
test("init out of order", should_fail=True)

try:
    rtc.init((year, month, day))
    test("init with only date", assert_time=False)
except ValueError as e:
    print(f"❌ Test failed: {e}")

try:
    print("\nTesting with too many arguments")
    rtc.init((year, month, day, hour, minute, second, microsecond, tzinfo, 1234))
    print("❌ Test passed but should have failed")
except ValueError as e:
    print(f"✅ Test failed as expected: {e}")

try:
    print("\nTesting with too few arguments")
    rtc.init((year, month))
    print("❌ Test passed but should have failed")
except ValueError as e:
    print(f"✅ Test failed as expected: {e}")

# The 8-tuple has the following format:
# (year, month, day, weekday, hours, minutes, seconds, subseconds)
rtc.datetime((year, month, day, weekday, hour, minute, second, microsecond))
test("datetime in order")

Output before the fix:

Testing init in order
(2024, 12, 27, 4, 6, 20, 0, 406)
❌ Test passed but should have failed

Testing init out of order
(2024, 12, 26, 3, 6, 30, 20, 258)
❌ Test should have failed but didn't
❌ Test failed: requested length 8 but object has length 3

Testing with too many arguments
✅ Test failed as expected: requested length 8 but object has length 9

Testing with too few arguments
✅ Test failed as expected: requested length 8 but object has length 2

Testing datetime in order
(2024, 12, 26, 3, 6, 30, 20, 218)
✅ Test passed

Output with the fix:

Testing init in order
(2024, 12, 26, 3, 6, 30, 20, 361)
✅ Test passed

Testing init out of order
(2024, 12, 26, 3, 0, 6, 30, 319)
✅ Test failed as expected: Hour should be 6 but was 0

Testing init with only date
(2024, 12, 26, 3, 0, 0, 0, 219)
✅ Test passed

Testing with too many arguments
✅ Test failed as expected: requested maximum length 8 but object has length 9

Testing with too few arguments
✅ Test failed as expected: requested minimum length 3 but object has length 2

Testing datetime in order
(2024, 12, 26, 3, 6, 30, 20, 249)
✅ Test passed

To allow for optional parameters but still checking the required tuple size range I added a new function that checks for the min and max size of the given tuple. I couldn't find an existing test suite for those functions. Any directions?

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:    +8 +0.002% PYBV10
     mimxrt:   -12 -0.003% TEENSY40
        rp2:    +0 +0.000% RPI_PICO_W
       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS
  qemu rv32:   +13 +0.003% VIRT_RV32

Thanks for the patch, and the comprehensive test.

Sorry that you ran into this issue. Indeed the docs don't match the behaviour, although it's not clear which is the "source of truth here" 😅

The mimxrt and samd ports also implement rtc.init() in the same way as the esp32 port (stm32 and renesas-ra have rtc.init() but it takes no arguments, it just resets the RTC peripheral). So either they all need changing, or the docs need changing.

Actually, PR #10607 attempts to fix all the affected ports, and fix some other inconsistencies. So I think that might be a good solution here?

I should also say that we do eventually want to fix this properly, by making all the methods follow the standard CPython tuple for date-time, namely (year, month, day, hours, minutes, seconds, milliseconds[, tzinfo]). That should hopefully reduce confusion because there will be only one way to specify the date-time.

For this change we either need to define completely new RTC methods that use this CPython tuple format, or wait for MicroPython 2.0.

The RTC.init() argument order is fixed by f4e4599

@sebromero would be good to get your test merged, so it can test this feature. And best if it was made generic, for any port, not just esp32. Do you want to attempt that? If not I can take it from here.

@dpgeorge Sure I can give it a shot. Some guidance needed though:

• Is it correct that we only test the C code by means of testing the uPython code that uses it behind the scenes?
• Is there a way to exclude the ports in the test that don't support RTC init with parameters? I could also try catch that case in the test but feels a bit quirky.
• Is there any value in keeping the mp_ob_get_array_min_max function that I created? It's meant to check for min/max tuple sizes in the case of a list of parameters with optional entries. I couldn't find such function, we only had mp_obj_get_array_fixed_n.
• Would you like me to reuse this PR or should I create a new one just with the test?

• Is it correct that we only test the C code by means of testing the uPython code that uses it behind the scenes?

Yes. Pretty much all tests are Python code running on the device (or unix port etc). And that Python code runs the underlying C code and tests it.

(One exception: the unix coverage build has coverage.c and coveragecpp.cpp that do C-level testing.)

• Is there a way to exclude the ports in the test that don't support RTC init with parameters? I could also try catch that case in the test but feels a bit quirky.

The best way is to use a SKIP mechanism at the start of the test, eg:

try:
    from machine import RTC

    RTC().init(0, 0, 0, 0, 0, 0, 0, 0)
except:
    print("SKIP")
    raise SystemExit

That is a little quirky but lots of other tests do this. Alternatively, for this case you know that the ports that don't conform are stm32 and renesas-ra` so you could do:

try:
    from machine import RTC
except:
    print("SKIP")
    raise SystemExit

import sys

if not hasattr(RTC, "init") or sys.platform in ("pyboard", "renesas-ra"):
    print("SKIP")
    raise SystemExit

You could also use unittest to implement the test, see eg tests/extmod/machine1.py.

• Is there any value in keeping the mp_ob_get_array_min_max function that I created?

Not at this stage.

• Would you like me to reuse this PR or should I create a new one just with the test?

It doesn't really matter.