pimoroni / enviro

MIT License
101 stars 79 forks source link

Enviro Weather Rain Function - count logic needs edit #81

Closed RedDogUK closed 1 year ago

RedDogUK commented 1 year ago

The current script counts the number of ticks in the last hour. Therefore if the board takes a reading every 15min (as defined by config.py), the rain readings would be duplicated or overstated in the upload files. I think the script should pull in the reading frequency variable from config.py and use that in the formula. Better yet, the board should only record the number of ticks between readings - even is a reading is manually initiated by using the "poke" button.

https://github.com/pimoroni/enviro/blob/027c776d1ec4e8ddd3acdd3f541199778c6297d0/enviro/boards/weather.py#L123

As a minimum, line 136 should if now - ts < config.reading_frequency ... I'll see if I can work on some test code and publish it via a fork if I get it to work.

the goal is if pouring 250ml of water from a jug over the rain sensor, the reading should reflect 250ml in the subsequent upload file and only in that file. Any subsequent file should have a "0" rain volume.

MrDrem commented 1 year ago

This is covered in #70, but may be better broken out and discussed here.

ZodiusInfuser commented 1 year ago

@RedDogUK @MrDrem Did either of you get chance to try out if now - ts < config.reading_frequency? I read the notes on #70 and it should be easy enough to convert the number of ticks into a mm per hour regardless of the time frame.

I feel like just using the reading frequency on its own isn't sufficient, as that assumes no other event woke up the board (like a poke for instance) in the meantime. So really the function needs to know when rainfall() was last called in order to determine the time base.

ZodiusInfuser commented 1 year ago

Okay, here's a possible solution

def rainfall():
  amount = 0
  now_str = helpers.datetime_string()
  if helpers.file_exists("last_time.txt"):
    now = timestamp(now_str)

    time_entries = []
    with open("last_time.txt", "r") as timefile:
      time_entries = timefile.read().split("\n")

    # read the first line from the time file
    last = now
    for entry in time_entries:
      if entry:
        last = timestamp(entry)
        break

    logging.info(f"  - seconds since last reading: {now - last}")

    if helpers.file_exists("rain.txt"):
      with open("rain.txt", "r") as rainfile:
        rain_entries = rainfile.read().split("\n")

      # count how many rain ticks since the last reading
      for entry in rain_entries:
        if entry:
          ts = timestamp(entry)
          if now - ts < now - last:
            amount += RAIN_MM_PER_TICK

  # write out adjusted rain log
  with open("last_time.txt", "w") as timefile:
    timefile.write(now_str)  

  return amount

This just returns the amount of rainfall since the last reading, but could also be modified to return the average rainfall since it knows the number of ticks and the time those ticks took place over.

Would having both values stored be useful, perhaps making the average the default to maintain compatibility with current behaviour, and having a separate rainfall_raw or something for what's above.

Also I do wonder if rain.txt should be wiped after the reading is taken 🤔

MrDrem commented 1 year ago

I would point out that as the rainfall has never worked fully on any code that I've seen (and certainly not on official code), whatever you do is compatible with current records, as none exist.

ZodiusInfuser commented 1 year ago

Haha, that is a very valid point! I've gone for rain being the rainfall in mm, and then added a rain_per_second which gives an average that can be scaled up to the original per hour if people want.

I would welcome some testing on this if either if you have a chance. Code is in https://github.com/pimoroni/enviro/tree/patch

ZodiusInfuser commented 1 year ago

There is a new release that fixes this issue: https://github.com/pimoroni/enviro/releases/tag/v0.0.9 I'll therefore close this but please re-open it or raise a new issue if you update and experience the problem again. Thanks