Circuitpython nickzoic 1046 nrf rtc #1534

nickzoic · 2019-02-12T02:44:02Z

This is an implementation of #1046 , with a couple of things missing:

I was having trouble with the weak references to common_hal_rtc_get_time and common_hal_rtc_set_time which I've got a workaround for but not really an explanation for
There's no calibration for this RTC. It could be calibrated by scaling the RTC counter values slightly but there's no facility for tuning the RTC clock itself.

dhalbert · 2019-02-12T03:52:57Z

I have been unable to get -flto to work on the nRF build, which is really strange (see #1396). I'm not sure if that might be related to the weak references issue, but possibly.

nickzoic · 2019-02-12T08:10:33Z

I'm very puzzled about the whole thing. I'm not, at this point, sure if it is a flaw in my understanding of __attribute__((weak)) or something genuinely weird going on, maybe in combination with inheriting the __attribute__((noreturn)) from the raise?

I got as far as working out that if I removed the weak declarations, OR changed the error messages raised by the weak declarations to be different, then the problem goes away. 8a88a6f uses this workaround so at least I could make some progress on this issue, but I'm not real happy about it.

nickzoic · 2019-02-12T08:13:52Z

(oh, and because I changed that error message it's upset the translations, which is what Travis is complaining about. I'm seeking a better way)

tannewt

This works great! Thanks! Please update your translations and it should be good to go.

Adafruit CircuitPython 4.0.0-beta.2-31-ga48d69501 on 2019-02-13; Adafruit Feather nRF52840 Express with nRF52840
>>> import rtc
>>> r = rtc.RTC()
>>> r.datetime
struct_time(tm_year=2000, tm_mon=1, tm_mday=1, tm_hour=0, tm_min=0, tm_sec=29, tm_wday=5, tm_yday=1, tm_isdst=-1)
>>> import time
>>> time.time()
946684843
>>> time.localtime()
struct_time(tm_year=2000, tm_mon=1, tm_mday=1, tm_hour=0, tm_min=0, tm_sec=57, tm_wday=5, tm_yday=1, tm_isdst=-1)
>>> r.datetime = time.struct_time((2019, 2, 14, 10, 20, 0, 3, 0, -1))
>>> time.localtime()
struct_time(tm_year=2019, tm_mon=2, tm_mday=14, tm_hour=10, tm_min=20, tm_sec=2, tm_wday=3, tm_yday=45, tm_isdst=-1)
>>> time.localtime()
struct_time(tm_year=2019, tm_mon=2, tm_mday=14, tm_hour=10, tm_min=20, tm_sec=10, tm_wday=3, tm_yday=45, tm_isdst=-1)
>>> time.localtime()
struct_time(tm_year=2019, tm_mon=2, tm_mday=14, tm_hour=10, tm_min=20, tm_sec=12, tm_wday=3, tm_yday=45, tm_isdst=-1)
>>>

ghost · 2019-02-16T19:06:18Z

Possibly this could be combined with deepsleep, which also needs an RTC timer as timeout and to advance the systick so time.monotonic() returns a reasonable result after sleep. This however would require a lower divide value (1024). My implementation of deepsleep is here: https://github.com/bboser/iotpython/blob/master/ports/nrf/common-hal/microcontroller/__init__.c.

The other possible issue is that apparently the softdevices (BT?) also use RTC timers. Somewhere I read that RTC0 is used by the softdevice but I cannot locate the reference.

tannewt · 2019-02-20T00:09:06Z

@nickzoic Are you blocked by anything on this?

nickzoic · 2019-02-20T12:39:20Z

Hi Scott! I got sidetracked trying to work out the weak reference thing. See comments at end of #1046.
Perhaps Dan's changes to make system will avoid this problem? It is very curious that it is a problem with -O2 set and not -O1, and very much repeatable.

nickzoic · 2019-03-02T11:22:58Z

OK so I've rebased it and sadly, no change. I've just changed one of the messages for now and provided translations as best I can approximate for the new message.

tannewt · 2019-03-08T20:09:24Z

I think we should get this in once the translations are merged. It should be good to go otherwise.

nickzoic · 2019-03-09T01:26:30Z

Did I get the push wrong again? I'm away for a few days but have laptop so can amend if necessary ...

…

-----Nick

tannewt · 2019-03-14T22:58:23Z

@nickzoic please take a look when you are back. There are files that need to be merged.

(works if MP_WEAK common_hal_rtc_get_time is removed)

the __weak linking works fine so long as these functions are not identical. I have not yet worked out why.

The mysterious MP_WEAK linking bug still exists, thus the new message for 'set'.

nickzoic · 2019-03-27T23:59:56Z

OK @tannewt trying yet again to get this to pass CI checks ...

dhalbert · 2019-03-28T00:01:52Z

@nickzoic Just saw this issue in micropython: micropython#4590. Is it relevant to your impl?

nickzoic · 2019-03-28T00:23:42Z

@dhalbert you mean for the problems with the Ethernet wing like #1500, right?
(I think there was another one too, or maybe that was a discussion on Discord)
I've never been able to successfully replicate those so haven't got anywhere tracking them down.

The Wiznet5K implementation for circuitpython doesn't use LWIP but it might be worth looking at the error handling in wiznet5k_socket_send() and see if there's situations there where we should retry rather than erroring out.

dhalbert · 2019-03-28T00:28:29Z

RIght - I didn't know how much of your code was common with the MPy code, if any.

tannewt

Tweaked one old comment and looks good otherwise. Will merge after Travis gives the ok. Thanks for your persistence on this!

nickzoic · 2019-03-28T23:01:35Z

On Fri, 29 Mar 2019, at 03:30, Scott Shawcroft wrote: ***@***.**** approved this pull request. Tweaked one old comment and looks good otherwise. Will merge after Travis gives the ok. Thanks for your persistence on this!

Thanks Scott! Yeah, getting languages to merge is a bit of a bear, eh ... every time I think it's working they get out of date again! (I do think multilingual support is a great feature though, I just wish it was easier to merge!)

tannewt · 2019-03-28T23:40:28Z

Ya, I looked into having a smarter merge but didn't see any great options.

tannewt · 2019-04-01T22:03:20Z

@nickzoic Want to take a look at why this broke BLE? Totally ok if you don't have the time.

ghost · 2019-04-01T22:13:15Z

The BLE soft device uses RTC 0. Just a hunch. See my earlier comment on github. Bernhard

…

On Mon, Apr 1, 2019 at 3:03 PM Scott Shawcroft ***@***.***> wrote: @nickzoic <https://github.com/nickzoic> Want to take a look at why this broke BLE? Totally ok if you don't have the time. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#1534 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AC3bpHcPztJUsooH1cstE8bS48SNVuiXks5vcoIsgaJpZM4a1e93> .

nickzoic · 2019-04-01T22:55:09Z

On Tue, 2 Apr 2019, at 09:13, Bernhard Boser wrote: The BLE soft device uses RTC 0. Just a hunch. See my earlier comment on github.

Yeah, I saw that one (thanks!) but didn't get around to following up (sorry!) A quick google suggests you're correct ... https://devzone.nordicsemi.com/f/nordic-q-a/5235/usage-of-rtc0-as-time-basis-system-time-possible https://devzone.nordicsemi.com/f/nordic-q-a/305/rtc0-rtc1-and-the-app_timer-with-softdevice https://devzone.nordicsemi.com/f/nordic-q-a/22941/how-to-enable-and-use-rtc0-rtc1-and-rtc2----nrf52832 Suggests RTC0 is used by the softdevice and RTC1 is used by the app_timer library which is used in other libraries. So maybe, so long as we're no longer targeting the '32 revision A, we should move it to RTC2? https://www.nordicsemi.com/DocLib/Content/Errata/nRF52832_EngA/latest/ERR/nRF52832/EngineeringA/latest/anomaly_832_10 I'll have to work out how BLE works so I can actually test this I suppose! Re: your other comments about deepsleep, definitely yes, that'd be a good idea.

…

-----N

(to avoid interference with Bluetooth Softdevice. See adafruit#1534 (comment) with thanks to @bboser for pointing it out)

nickzoic · 2019-04-02T00:46:40Z

OK so I've changed it across to RTC2 and it works from the point of view of the RTC, has anyone got some code which demonstrates it not working with BLE which would work with BLE stuff I might have lying around?
(@bboser @tannewt @dhalbert)

dhalbert · 2019-04-02T00:52:55Z

See the example in #1726. Try that. If that doesn't cause an immediate error, then doing something like https://github.com/adafruit/Adafruit_CircuitPython_BLE/blob/master/examples/ble_uart_echo_test.py with the Adafruit Bluefruit LE Connect app on the other end would be a simple test.

dhalbert · 2019-04-02T01:01:33Z

Also, take out the MP_WEAK attributes on the functions. That should not be necessary. If they ever were, they are not needed now.

nickzoic · 2019-04-02T02:04:08Z

OK that UART example works with nickzoic/micropython@48f056647f which now uses RTC2. Thanks for the pointer, I hadn't tried using BLE for anything yet.

I'll rebase onto master and make a new PR very soon

(to avoid interference with Bluetooth Softdevice. See adafruit#1534 (comment) with thanks to @bboser for pointing it out)

dhalbert · 2019-04-02T02:40:32Z

Yes, we don't compile the source files for modules that are turned off. But rtc.RTC was the only one that ever used MP_WEAK. I'm not sure why it was done that way to begin with. If you grep for MP_WEAK, you'll find very few uses. You don't need the NotImplemented versions. But a use or two here or there may need to check #if CIRCUITPY_RTC.

Or maybe I am all wrong about this, but looking at the uses of RTC, it does not seem to need fallback routines.

nickzoic · 2019-04-02T02:52:05Z

Yep, I've removed them altogether since both atmel-samd and nrf ports have RTC anyway.

Thanks for your patience :-) #1736 should hopefully work out better!

tannewt self-requested a review February 13, 2019 18:33

tannewt previously approved these changes Feb 14, 2019

View reviewed changes

tannewt added this to the 4.0.0 - Bluetooth milestone Feb 14, 2019

tannewt added enhancement nordic circuitpython api labels Feb 14, 2019

dhalbert mentioned this pull request Feb 17, 2019

Added DS3231 RTC Featherwing adafruit/Adafruit_CircuitPython_FeatherWing#27

Merged

makermelissa mentioned this pull request Feb 17, 2019

RTC Functionality missing from NRF making DS3231 Library Unusable #1561

Closed

nickzoic dismissed tannewt’s stale review via f04294f March 2, 2019 11:21

nickzoic force-pushed the circuitpython-nickzoic-1046-nrf-rtc branch from a48d695 to f04294f Compare March 2, 2019 11:21

tannewt modified the milestones: 4.0.0 - Bluetooth, 4.x Mar 14, 2019

nickzoic added 8 commits March 28, 2019 09:50

starting on micropython#1046 rtc for nRF

77f307c

more fake RTC code ... adafruit#1046

69cf33e

(works if MP_WEAK common_hal_rtc_get_time is removed)

enable NRFX RTC adafruit#1046

b09d2c3

workaround for problem with adafruit#1046

ff6395f

the __weak linking works fine so long as these functions are not identical. I have not yet worked out why.

adafruit#1046 handle overflows in the RTC counter

28254de

There isn't really a good way to calibrate this RTC adafruit#1046

71622a4

Fix up messages & wild stab at translations for adafruit#1046

9368473

The mysterious MP_WEAK linking bug still exists, thus the new message for 'set'.

Update translations (again)

a06ce33

nickzoic force-pushed the circuitpython-nickzoic-1046-nrf-rtc branch from c297266 to a06ce33 Compare March 27, 2019 23:06

nickzoic mentioned this pull request Mar 28, 2019

Improve rtc documentation #1563

Closed

nickzoic and others added 4 commits March 28, 2019 14:47

Reset translations to master

02f1939

Merge branch 'master' into circuitpython-nickzoic-1046-nrf-rtc

54a41e0

re-run make translate

5b98102

Update comment

98811a9

tannewt approved these changes Mar 28, 2019

View reviewed changes

tannewt self-assigned this Mar 28, 2019

tannewt merged commit a30bbe6 into adafruit:master Mar 28, 2019

dhalbert mentioned this pull request Mar 29, 2019

Revert "Circuitpython nickzoic 1046 nrf rtc" #1726

Merged

nickzoic added a commit to nickzoic/micropython that referenced this pull request Apr 2, 2019

Change nRF RTC implementation to use RTC2 micropython#1046

22f0c00

(to avoid interference with Bluetooth Softdevice. See adafruit#1534 (comment) with thanks to @bboser for pointing it out)

nickzoic added a commit to nickzoic/micropython that referenced this pull request Apr 2, 2019

Change nRF RTC implementation to use RTC2 micropython#1046

94bda3b

(to avoid interference with Bluetooth Softdevice. See adafruit#1534 (comment) with thanks to @bboser for pointing it out)

Circuitpython nickzoic 1046 nrf rtc #1534

Circuitpython nickzoic 1046 nrf rtc #1534

Uh oh!

Conversation

nickzoic commented Feb 12, 2019

Uh oh!

dhalbert commented Feb 12, 2019

Uh oh!

nickzoic commented Feb 12, 2019

Uh oh!

nickzoic commented Feb 12, 2019

Uh oh!

tannewt left a comment

Choose a reason for hiding this comment

Uh oh!

ghost commented Feb 16, 2019

Uh oh!

tannewt commented Feb 20, 2019

Uh oh!

nickzoic commented Feb 20, 2019

Uh oh!

nickzoic commented Mar 2, 2019

Uh oh!

tannewt commented Mar 8, 2019

Uh oh!

nickzoic commented Mar 9, 2019 via email

Uh oh!

tannewt commented Mar 14, 2019

Uh oh!

nickzoic commented Mar 27, 2019

Uh oh!

dhalbert commented Mar 28, 2019

Uh oh!

nickzoic commented Mar 28, 2019

Uh oh!

dhalbert commented Mar 28, 2019

Uh oh!

tannewt left a comment

Choose a reason for hiding this comment

Uh oh!

nickzoic commented Mar 28, 2019 via email

Uh oh!

tannewt commented Mar 28, 2019

Uh oh!

tannewt commented Apr 1, 2019

Uh oh!

ghost commented Apr 1, 2019 via email

Uh oh!

nickzoic commented Apr 1, 2019 via email

Uh oh!

nickzoic commented Apr 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dhalbert commented Apr 2, 2019

Uh oh!

dhalbert commented Apr 2, 2019

Uh oh!

nickzoic commented Apr 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dhalbert commented Apr 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nickzoic commented Apr 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

nickzoic commented Apr 2, 2019 •

edited

Loading

nickzoic commented Apr 2, 2019 •

edited

Loading

dhalbert commented Apr 2, 2019 •

edited

Loading

nickzoic commented Apr 2, 2019 •

edited

Loading