8000 Support for STM32 architecture + STM RTC clock (take#2) by arkhipenko · Pull Request #39 · bxparks/AceTime · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Support for STM32 architecture + STM RTC clock (take#2) #39

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Dec 9, 2020
Merged

Support for STM32 architecture + STM RTC clock (take#2) #39

merged 2 commits into from
Dec 9, 2020

Conversation

arkhipenko
Copy link
Contributor

Hi,
this is take 2 after screwing up the branches - should be ok now and off the latest develop branch.

  • Support for STM32 architecture (compat.h)
  • STMRTC hardware abstraction
  • StmRTC clock - a clock based on embedded RTC on STM chips. Tested on STMF103C8

Cheers and thank you for this extremely useful library!
Anatoli

@bxparks
Copy link
Owner
bxparks commented Dec 8, 2020

The content looks good. But you have converted all these files into DOS format with (\r\n) instead of (\n) line termination. So the git diff is massive, since each file was completely replaced by your version:

$ git show --stat
commit f032283286b1d50b1901673f5c184add4a27e3fd (HEAD -> arkhipenko-develop)
Author: arkhipenko <arkhipenko@hotmail.com>
Date:   Tue Dec 8 08:23:33 2020 -0500

    Support for STM32 architecture + STM RTC clock

 keywords.txt                     | 634 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------------------------------
 src/AceTime.h                    | 135 +++++++++++++-------------
 src/ace_time/clock/StmRtcClock.h |  79 +++++++++++++++
 src/ace_time/common/compat.h     | 214 ++++++++++++++++++++++-------------------
 src/ace_time/hw/StmRtc.cpp       |  77 +++++++++++++++
 src/ace_time/hw/StmRtc.h         |  50 ++++++++++
 6 files changed, 709 insertions(+), 480 deletions(-)

Can you fix your git config and try again? This may help you:
https://stackoverflow.com/questions/10418975/how-to-change-line-ending-settings
Looks like you are on Windows, so you want to do:

git config --global core.autocrlf true

@arkhipenko
Copy link
Contributor Author

I switched to Linux specifically to fix the filenames as the windows version would not pick up the difference.
Now the CR/LF issue. Git for windows was converting automatically, Linux version didn't.
Trying one more time, and will give up after this one. Sorry - it ends up requiring too much time.

@bxparks
Copy link
Owner
bxparks commented Dec 8, 2020

If you can't fix it, please keep this PR open and I'll find a way to merge this code in properly myself. I got some STM32 on order, so I should be able to actually test things too.

It sounds like it might be worth tracking down these git config problems on your Windows or Linux machine. In theory, git should be doing these NL/LF conversions for you automatically for text files, so these problems shouldn't happen.

@arkhipenko
Copy link
Contributor Author

It sounds like it might be worth tracking down these git config problems on your Windows or Linux machine. In theory, git should be doing these NL/LF conversions for you automatically for text files, so these problems shouldn't happen.

Git for Windows was doing CR/LF conversion properly, but missed the filename changes...
Git for Linux picked up the filenames properly, but then explicitly changed all LFs to CR/LF during git add. (Could be how the git config --global core.autocrlf true command actually works...
Something is wrong, but this is the first time I hit this issue. (First time had to change case in the filenames either...........).
Anyway - hope you can make this work. I am definitely using it in my project already.
Thanks again for the library - very comprehensive and easy to use (once you understand how to :)) )

@bxparks
Copy link
Owner
bxparks commented Dec 8, 2020

Success! I think you fixed the CR/LF problem. I see a few trailing whitespace nits, but I can fix those later. Everything else looks great. Since I got the STM32s on order, I think I'll wait until they arrive so that I can test this code myself. Thanks for the PR! (If I don't update this within about a month, you can ping me to see what's up.)

@arkhipenko
Copy link
Contributor Author

Great. Thanks.

Q: for production, shall I compile against your master or develop branch?
Also, which branch is being updated with the recent TZ and how often?

@bxparks
Copy link
Owner
bxparks commented Dec 8, 2020 •
edited
Loading

The trunk for all development is develop. I try to keep all the unit tests passing, but it may not be the most stable. Just before releasing, I merge into master and create a release tag there. The version you see on the Arduino Library Manager is master. When a new TZ version is released, I create a release within a week if not sooner. [Edit: typo]

@bxparks
Copy link
Owner
bxparks commented Dec 9, 2020

Actually, to avoid dealing with conflicts later, I am going to merge this intodevelop right now and mark it as "Experimental". A bunch of stuff under examples/ and tests/ are having some issues with the stm32duino core, but I think I managed to get those compiling. Clearly I cannot actually run anything, but at least they compile.

bxparks added a commit that referenced this pull request Dec 9, 2020
@bxparks
tests: Add env:stm32 in auinter.ini, and fix assertEqual(), prep for …
61ab656 
…merging in STM32 code from #39
@bxparks bxparks merged commit ab261b8 into bxparks:develop Dec 9, 2020
bxparks added a commit that referenced this pull request Dec 9, 2020
@bxparks
src: Fix minor formatting issues with STM32RTC code (update to #39)
15bc4dd
@bxparks bxparks mentioned this pull request Dec 31, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@arkhipenko @bxparks
0