Closed adamgarbo closed 4 years ago
A quick note that the failed checks are due to: fatal error: WDT.h: No such file or directory
Is this perhaps due to using #include <WDT.h>
instead of #include "WDT.h"
in the examples?
So yes, I've been through and commented, I thought that was just on the files, but it's made a whole lot of comments on the main conversations - any Github tips?
Basically - excellent work, I only had suggestions for:
Thanks @stephenf7072!
I believe I've addressed all of your comments in 38a241b. Also, please see your individual comments for more detailed information of specific issues.
Cheers, Adam
Nice one Adam. I've been over the new bits and apart from the minor omission of a "HZ" in keywords, no further comments from me.
Hi @adamgarbo and @stephenf7072
Thanks for all your effort! You both probably know that we are in the midst of a big transition to the v2.0.0 core. That being said I think this PR should be merged into v2.0.0 rather than trying to squeeze it in on the tail end of v1.x.x
This PR is not forgotten!
Thanks @oclyke
Is there a way to redirect which branch this PR will merge with? Please let me know if there's anything I can (or should) do.
Also, would you like me to create a new RTC library PR for v2.0.0 with the new time variable order (i.e. hund, sec, min, hour, etc.)?
Cheers, Adam
@adamgarbo I have already put the RTC library w/ the order changes into the v2.0.0 branch (took the liberty to do so cause I didn't want to bug anyone for help with it)
~Yes, I will change the target of this PR - then perhaps I can get around to testing it and make it part of v2.0.0 rather than 2.x.x~
Edit: actually I think you need to change the target branch. Should be fairly simple / intuitive.... lmk if you have trouble
@oclyke Well, that was easy! Updated base branch to v2.0.0. I assumed that was the correct one, and not the preview?
Yup! There is a sort of rolling release going on like this:
v2.0.0
spoof-dev
to trigger CI buildsspoof-release-candidate
into v2.0.0 to be back on trackv2.0.0
in v2.0.0-preview
for people to use w/o my changes breaking everythingBy the way - there will be a decent amount of work to do to make this compatible with v2.0.0
- there is a completely different file structure and philosophy. I'll be here to lend a hand
Thanks, Owen.
Is there any documentation on the new file structure and philosophy? Keen to learn more and happy to make changes as necessary.
Cheers, Adam
A quick glance at the new diffs makes me think..... it would probably be easiest to start from scratch based on the v2.0.0 branch. You could of course make a local copy of the meat + potatoes files of your PR.
Wow - you're quick. Didn't see your last request for docs.
I don't have anything official to hand you. Maybe you will be able to tell just by looking at the file structure. Also possible that you won't be affected if you are just writing a library. Here are some notes:
run git submodule update --init --recursive1
in the core repo to make sure all the submodules exist for you
standard Arduino API functions that can be handled by mbed go in cores/arduino/mbed-bridge
(which is its own repo)
standard Arduino API functions that rely on the Ambiq HAL go in cores/arduino/sdk
generally I am matching implementation files with their respective header files - even when it means having two c
or cpp
files with the same name in different core locations
license notices are shortened and just point at the LICENSE.md file in the core
There's probably a lot more - that's all for now though
@adamgarbo - we've decided to do another support release on the v1.x core, so I will test this out and try to pull it in. That way we can have a reference for what we should be doing on the v2.0.0 version
Hi @oclyke,
Sounds good! Please let me know if you need any assistance or clarification.
I haven't yet had a chance to rewrite the WDT library, but if the v2.0.0 RTC library is any example, it shouldn't be too much work. I noticed some of the changes you mentioned (e.g. #ifndef _APOLLO3_LIBRARIES_RTC_H_
and class Apollo3RTC {}
).
I'll try to submit a PR to v.2.0.0 in the next couple of days. What's your optimistic timeframe for its release?
Cheers, Adam
@adamgarbo I just tried out these examples. First of all (I hope I have said this before) thanks for the great work! Your examples are well laid out and straight to the point.
The first example worked well. I then went on to test examples 2 and 3, but saw no output. Stumped, I went back to example 1 (basic) and saw no output any more. Moving on I even went to an unrelated example (pure serial) I still saw no output.
I haven't looked into it any deeper (using a debugger, for example) but can you confirm that you do not have similar issues?
Hi Owen,
I initially tested all examples on a SparkFun Artemis Redboard. I just now tested them on an Edge 2 and they all appeared to be working correctly. Please see below for their respective outputs:
Example 1
15:18:15.045 -> Artemis Watchdog Timer Example
15:18:19.215 -> Interrupt: 1 Period: 4122 ms
15:18:23.277 -> Interrupt: 2 Period: 4067 ms
15:18:27.386 -> Interrupt: 3 Period: 4065 ms
15:18:31.429 -> Interrupt: 4 Period: 4067 ms
15:18:35.523 -> Interrupt: 5 Period: 4064 ms
15:18:35.523 -> Warning: Watchdog has triggered a system reset
Example 2
15:18:55.097 -> Artemis Watchdog Timer Example
15:18:56.357 -> Interrupt: 1 Period: 1007 ms
15:19:00.393 -> Interrupt: 2 Period: 4019 ms
15:19:04.480 -> Interrupt: 3 Period: 4066 ms
15:19:08.549 -> Interrupt: 4 Period: 4066 ms
15:19:12.639 -> Interrupt: 5 Period: 4068 ms
15:19:16.714 -> Interrupt: 6 Period: 4064 ms
15:19:20.790 -> Interrupt: 7 Period: 4064 ms
15:19:24.855 -> Interrupt: 8 Period: 4065 ms
15:19:28.954 -> Interrupt: 9 Period: 4065 ms
15:19:28.954 -> Warning: Watchdog has triggered a system reset
Example 3
15:20:16.623 -> Artemis Watchdog Low Power Example
15:20:16.877 -> ⸮Watchdog interrupt: 2020-08-01 00:00:10.11
15:20:27.015 -> ⸮Watchdog interrupt: 2020-08-01 00:00:20.15
15:20:37.042 -> ⸮Watchdog interrupt: 2020-08-01 00:00:30.20
15:20:47.074 -> ⸮Watchdog interrupt: 2020-08-01 00:00:40.26
15:20:57.122 -> ⸮Watchdog interrupt: 2020-08-01 00:00:50.31
15:21:07.192 -> ⸮Alarm interrupt: 2020-08-01 00:01:00.01
15:21:16.872 -> ⸮Watchdog interrupt: 2020-08-01 00:01:10.04
I could have sworn we fixed that issue with the Artemis going to sleep and waking up without producing a question mark in the serial output. @nseidle did we only fix this on the OpenLog Artemis?
Cheers, Adam
Thanks - I will look more closely at my setup to see if something is going wrong.
I also thought we had fixed the UART startup issue. If I recall correctly it had to do with configuring the UART pins after the UART was initialized...
Alright, I think I found the issue and it is totally unrelated to this library. It is described in issue #254
I don't believe this is at all related to this PR, and it doesn't occur with the SVL loader either (hence why most people are unlikely to experience the issue)
Hi @oclyke,
After our recent discussions regarding #237 by @stephenf7072 , I was motivated to get the ball rolling with writing a WDT library the Apollo3 Core. I started working with the WDT HAL earlier this year, and have been meaning put a library together for some time.
Library summary: For the most part, this library is simply a wrapper of the Ambiq HAL. As we know, the HAL can be scary, so the watchdog functions can now be called using basic commands such as
start
,stop
,restart
, etc.Where I needed to bypass the HAL and switch to writing registers directly was with the watchdog configuration functionality. Normally, when configuring the WDT with the HAL, you need to create a specific
am_hal_wdt_config_t
structure, specify your clock divider and tick values and then pass this structure to the HAL functionam_hal_wdt_init
, which actually does the configuration. Rather than dealing with structures, addresses and pointers, I opted to write a few of simple functions (e.g.configure()
,setClock()
,setInterrupt()
,setReset()
, to have greater overall control of these registers.Default configuration: There is a globally declared
am_hal_wdt_config_t watchdogConfig
structure in the header file, which is used for the initial WDT configuration. This defaults the LFRC clock divider to 16 Hz, the interrupt period to 4 seconds and reset period to 8 seconds. For users of either Atmega328p or SAMD21 watchdogs, these periods will be familiar, though are ultimately arbitrarily and able to be changed.All examples tested with a SparkFun Redboard Artemis. A test program (not included) was also run to iterate the WDT through every clock divider, interrupt and reset tick combination to ensure correct timings.
My hope is that other users will identify additional required functionality or make contributions themselves. I welcome your feedback!
Cheers, Adam
Examples:
1. Example1_WDT_Basic
2. Example2_WDT_Config
3. Example3_WDT_LowPower