Skip to content
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.

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

Pr develop time audit #342

Closed
wants to merge 3 commits into from
Closed

Conversation

neilh10
Copy link
Contributor

@neilh10 neilh10 commented Dec 16, 2020

This PR makes changes for time functions to be either using UTC or Tz (local time offset).
#310
#304

This is tested on the basis that internal time is UTC – used by the hardware RTC and internal time keeping.
All USER interface is then supplied with UTC+localTimeOffset ~ though there is some judgement call as to what is a user interface.
This is primarily the uSD file times, and recording the sensor time as localTime-Offset.
This PR is also a first pass of learning how to test changes before submitting to the (develop) branch.
It's anticipated that the same method will be used for testing any new functionalty for
#194

This has been tested with a specific build created in sensor_tests/referenceTest/
To use from a working platformIO file-->open folder --> ModularSensors\sensor_tests\referenceTest
And then build.

There is a minor addition of functionality for dealing with SAMDxx series/extRtcPhy. Real SAMD sleep is a lot more complex than this PR ~ but I’d be happy to discuss it or submit what I’ve found works on a SAMDxx.

It should be possible to checkout this (pr_develop_time_audit) as a branch, and rebuild and then test this PR.

This was tested in this way before submitting.
(remove battery so clock is not maintained, and turn off board before testing)

  • On boot it, it collects NST time and updates Mayfly RTC if needed. If internal clock is strange value it is indicated before it is update to the NIST time.
  • On file creation <local_date> is used for creating file - refTest_uSD_2020-12-15.csv
  • It sets up the uSD creation time as local time
  • On writing a record it uses local time with offset (PST offset)
  • It updates the uSD file time to be the write time.

LogFile of TTY output
tty201215-1852prTimeAudit2.txt

LogFile on uSd

refTest_uSD_2020-12-15pr2_time.csv.txt

@neilh10
Copy link
Contributor Author

neilh10 commented Dec 16, 2020

shoot ~ I meant to take out GETNOWEPOCH_FN & SETNOWEPOCH_FN - it was about testing to make sure I had caught all references. I can take them out or anyother comments you have. :)

@neilh10
Copy link
Contributor Author

neilh10 commented Aug 31, 2021

I guess this is really out-of-date.
I'm very discouraged that this had no comment.
IHMO Internal "time" tracking is confusing, and could do with refactoring.

@aufdenkampe
Copy link
Member

Hi @neilh10, sorry to not look at this closer. It's been quite a busy year, which I think was especially true for @SRGDamia1 as she prepared for and started maternity leave.

@neilh10
Copy link
Contributor Author

neilh10 commented Sep 3, 2021

Ok thanks. I hadn't heard about maternity leave for SRGDamia1, though I guessed she was focused on other areas. :)
Happy to wait till she is doing well and has some time to share.
Overall I'm just not too sure how to contribute what I've developed and tested back to the main repository if anything.
There is an indication of #194 being desired, and I think of this as a dependency to it.

@neilh10
Copy link
Contributor Author

neilh10 commented Apr 9, 2022

I guess I'm doing this wrong.
I submitted this 16months ago ... limited discussion about it.

Really appreciate that some of these suggestions have been include in https://github.com/EnviroDIY/ModularSensors/releases/tag/v0.33.0

However, I really find it challenging to figure out how to try and make suggestions for improvements.

Is writing it up as an issue better, instead of doing all the work for a PR?.
Doing a PR can be a lot of extra work.

@neilh10 neilh10 closed this Apr 9, 2022
@neilh10 neilh10 mentioned this pull request Apr 9, 2022
@SRGDamia1
Copy link
Contributor

I'm really sorry, I really, really appreciate your work on testing the code. I'm sure your version is much better tested than mine! Unfortunately, I'm a solo developer with other projects and a bad tendency to flip between "put out the biggest fire" mode and perfectionism. I have several items on my to do list with notes like "Neil has already figured this out; merge his work." If other things would slow, I very much want to pull in your systems for queuing and retrying posts and working with configuration files. I'm also thinking about what it would take to support the esp32 as the main brain since the world's supply of 1284p chips seems to be exhausted. It would take some significant changes to deal with the sleep and interrupts, but would dramatically increase the number of devices that could run the code and give boatloads more power and storage.

@aufdenkampe
Copy link
Member

@neilh10, I would like to echo the comments and apology from @SRGDamia1.

Both of us are unfortunately in a situation where it's very difficult to carve out time for anything but the most pressing demands from the projects that are paying us, so lots of things that we would really like to do just slip from our plates.

You can this by the many issues that @SRGDamia1 and I create ourselves, but then often take years to address.

I'm very interested in your several Pull Requests, including this one. Sorry to not engage in this conversation sooner.

I'm going to reopen this issue so that it remains on our TO DO list.

@aufdenkampe aufdenkampe reopened this Apr 14, 2022
@neilh10
Copy link
Contributor Author

neilh10 commented Apr 14, 2022

Hi @aufdenkampe and @SRGDamia1 thanks for the comments.

I do appreciate there is a lot of juggling - and not unusual in our profession - and I'm just trying to identify ways that I can co-operate, contribute back in a way that is less likely to cause headaches, as well as go for my targets.

I added some more comments when cleaning up and closed this - #310

Seems to me there is a bigger discussion to be had - from the older generation maxed out mega1284-->??? ESP32 (have you seen amazon.com/dp/B08VGRZYJR/) or WioTerminal (SAMD51 https://www.amazon.com/gp/product/B087LNFZ2T) - low cost reliable powering from 4400mA, and reliable multiple SDI-12/RS485 instruments at +12V.

@aufdenkampe - I believe the intent of this PR has been completed with the latest release , and is now so out-of-date that it doesn't contribute to the intent. If you agree can you close it. I have merged all 0.33.0 changes to my fork so I'm in sync with the base.

https://github.com/EnviroDIY/ModularSensors/releases/tag/v0.33.0
Breaking: Renamed the static markedEpochTime variable to markedLocalEpochTime.
This was sometimes used in "complex" loops. Code utilizing it will have to be changed.
This is part of the effort to clarify where localized and UTC time are being used.
We recommend a logger's real time clock always be set in UTC and then localized for printing and storing data.
Breaking: Renamed the function setNowEpoch(uint32_t) to setNowUTCEpoch(uint32_t).
Although public, this was never intended to be used externally.

@SRGDamia1
Copy link
Contributor

Yup, big discussion on processors. @s-hicks2 has had to redraw everything so many times because of availability issues.

I had not seen the WioTerminal; it looks pretty spiffy. It just needs solar and a battery backed RTC. Have you grabbed one? I think most code is readily portable from the SAMD21 to SAMD51 so ModularSensors should run just find on it.

The ESP32 is a bit of a name out of a hat; but it seems to be winning a lot of popularity contests. We've had XBee footprint ESP32's made to use as WiFi modems - and they work well for it - but it's a much smarter chip than the 1284p and just as easily programmed so turning it into a dumb AT slave seems like a waste.

@neilh10
Copy link
Contributor Author

neilh10 commented Apr 14, 2022

@SRGDamia1 I'm playing with a WioT & battery terminal - neilh10#109

@neilh10
Copy link
Contributor Author

neilh10 commented Dec 1, 2022

Effectively merged - my branch is synced with the changes.

@neilh10 neilh10 closed this Dec 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants