add support for timer and clkout

[bablokb]

This add support for the builtin timer and for signal generation using the CLKOUT-pin

[tannewt]

Thanks for the PR! Any idea how much memory these additions take? I wonder if we should add them with a subclass.

[bablokb]

Is gc.mem_free() exact enough to answer this question?

[tannewt]

Yup! Please call gc.collect() before it.

[bablokb]

Test code is:

import gc
import board
import busio

PIN_SDA  = board.GP2
PIN_SCL  = board.GP3

gc.collect()
print("free mem before import: %s" % gc.mem_free())

from adafruit_pcf8563 import PCF8563
gc.collect()
print("free mem after import: %s" % gc.mem_free())

i2c = busio.I2C(PIN_SCL,PIN_SDA)
rtc = PCF8563(i2c)
gc.collect()
print("free mem after constructor: %s" % gc.mem_free())

The new implementation uses 1840 extra bytes (after object creation), comparing py-files.

BTW: pressing repeatedly CTRL-D to reload the program will result in different results. So the state does not seem to be reset deterministically.

I could refactor the timer-code to a separate class and add a get_timer() method. The timer-class would then implement all the timer-specific register access. Note that I have a similar pull-request for the PCF8523: adafruit/Adafruit_CircuitPython_PCF8523#30 which then should probably also be implemented like this.

[tannewt]

BTW: pressing repeatedly CTRL-D to reload the program will result in different results. So the state does not seem to be reset deterministically.

How much does it vary?

I could refactor the timer-code to a separate class and add a get_timer() method. The timer-class would then implement all the timer-specific register access.

I would add a separate timer class in a separate file (in the same file it'll still be loaded into ram.) You could then have that class subclass the regular one or take it into the constructor. I wouldn't have the main class construct the timer class for you.

Note that I have a similar pull-request for the PCF8523: adafruit/Adafruit_CircuitPython_PCF8523#30 which then should probably also be implemented like this.

👍

[bablokb]

How much does it vary?

As far as I remember about 1K.

I would add a separate timer class in a separate file (in the same file it'll still be loaded into ram.) You could then have that class subclass the regular one or take it into the constructor. I wouldn't have the main class construct the timer class for you.

The timer-class should go into a separate class for sure. But I don't see any necessity to subclass the regular one or even use an instance of it. Both the rtc-class and the timer-class are more or less facades for specific registers of an I2C-device with fixed address and the timer-class would not share or even depend on anything of the rtc-class (n.b. the timer doesn't even need a correct time to be set in the rtc).

I will post an updated version of the PR for discussion.

[tannewt]

I will post an updated version of the PR for discussion.

Sounds good! That's a good point.

As far as I remember about 1K.

Weird. That seems like a lot. I'd expect it to be pretty deterministic.

[bablokb]

This refactors the timer-code to it's own class. Probably the pyproject.toml needs some care too. Normally I would expect multipe files/classes within module (i.e. the adafruit_pcf8563 should be a directory), but that would break existing code.

[bablokb]

The CI tests fail now because of two toplevel files and recommend to merge them into a single file. Or to create a subdirectory). But as noted above, this would break the imports of existing code.

[FoamyGuy]

I think making the subdirectory so that adafruit_pcf8563/ is a directory instead of .py file is the way to go.

It will cause a change to the import statement in code that uses it, but we can make sure to update all of our example code at the same time, and release it with a major version number increase to indicate the incompatibility.

Having two python library files in the root of the repo causes issues with other parts of the infrastructure and surrounding tools. Many of them assume it will either be single file, or folder with base name and multiple files inside.

[bablokb]

I fully agree. Note that a similar PR for the 8523 would probably affect far more people because of the existing PCF8523 breakout. But I don't think there is a better solution.

[bablokb]

Any ideas on how to reach a consensus on what to do?

[tannewt]

I agree with @FoamyGuy's suggestions.

[bablokb]

Ok, fine. So I will post a new version of the PR moving the files to a subdirectory. Once that is fine, I will also rework the PR for the 8523.

[bablokb]

I don't understand the problem of the CI, it states

docstring of adafruit_pcf8563.pcf8563:33:Enumerated list ends without a blank line;

but there is a blank linke after line 33 and it is also an area I did not touch.

[jposada202020]

@bablokb I created bablokb#1 with the necessary changes. you coudl either change it in your branch or merge my pull request in your branch. sphinx is sometimes... well sphinx :)

[bablokb]

Is there a reason (except lack of time) why this PR is not merged?

[jposada202020]

Sorry about that, I could merge. @tannewt and @FoamyGuy any comments?

[tannewt]

Sorry, this fell off my radar. Feel free to ping me if I haven't replied in over a business day.

One suggestion. Looks good otherwise and will need to be a major release.

[bablokb]

I have removed the clokout-attributes with one exception: I left clokout_enabled. This is because the CLKOUT is active on POR, but the PCF85x3 user's manual recommends to disable it to minimize power-consumption.

[tannewt]

Looks good! Thank you!