Different limiting strategies

[ZeroOne3010]

Currently, by default, the app records new measurements every ten seconds. There should of course be some limiting strategy like this, as otherwise one would fill their database too quickly.

However, if you intend to track something that takes less than 10 second, such as some movements, there's a great chance you won't catch the packet(s) where the accelerometer values are interesting.

So I propose that the application should have different methods of limiting the number of saved measurements. Maybe the app could decide to store a measurement after all even if 10 seconds hasn't elapsed yet, provided that some threshold of some sensor values are exceeded. Or maybe it should remember, say, the maximum values of the 10 second period and record those at the end of it.

I volunteer myself for working on this improvement as I've already started on it in my own fork. :)

[Scrin]

This has actually been on my todo-list for ages, near the bottom though as InfluxDB is insanely good in compressing the data fast and efficiently. Personally I store every measurement received (limit 0), so far I have stored almost 60 million measurements, consuming 5.9GB of disk space. Though on a RPi with local InfluxDB and a 8GB card it's not that simple. :)

That being said, I'll be happy to accept a PR implementing this feature, as long as it doesn't break anything existing (I don't want to create a new "breaking changes" release yet, as I have something bigger in the works that will require that anyway).

My suggestions/comments (in case you didn't already think about these):

• Keep the current "static time limit" as the default mode (to avoid breaking existing "default config setups", in a later release with other breaking changes I may switch the default if the dynamic limiting works well)
• Make all of the thresholds and other tunables configurable via the config file (the "advanced" section at the bottom is probably the right place for these, though choosing the limiting mode itself probably fits best near the current update limit setting in the example config)
• Create a new package fi.tkgwf.ruuvi.strategy and fi.tkgwf.ruuvi.strategy.impl and separate the current limiting logic there (shouldUpdate methods in the handlers, in the current ugly way), and add this new dynamic limiting logic there as well (rather than making any additional changes to the handlers, this is mainly because https://github.com/Scrin/ruuvitag-common-java will replace the current handler classes once the lib is stable enough)
• Instead do the filtering in the main, after the handlers have parsed (rather than the handlers filtering anything), maybe here: https://github.com/Scrin/RuuviCollector/blob/f2f009e0273dd7caa6fa21a0a913ed6e672212c2/src/main/java/fi/tkgwf/ruuvi/Main.java#L103

How does this sound? Any comments?

[ZeroOne3010]

Thank you for your detailed response! I have indeed been thinking along the same lines. You might want to check out my current fork to see how it's going: https://github.com/ZeroOne3010/RuuviCollector/commits/dev I'm working in really small increments, so while the pull request might eventually be on the large side, one should be able to easily read it commit by commit.

I started by refactoring the current implementation so that it can be properly tested. Then I wrote an integration test that covers the entire process from reading hcidump to storing the results, and asserted the way that the application currently works. Then I just finished moving the current shouldUpdate logic out of the handlers, and lo and behold, I did not have to touch my integration test at all: it just passed. :)

[ZeroOne3010]

I finished extracting the current limiting strategy into an interface and an implementation. Applying the strategy now looks like this: https://github.com/ZeroOne3010/RuuviCollector/blob/5dbd646db50cb4be75f971adc3b6cb5b1ba0c682/src/main/java/fi/tkgwf/ruuvi/PersistenceService.java#L27-L29

And the new interface looks like this: https://github.com/ZeroOne3010/RuuviCollector/blob/5dbd646db50cb4be75f971adc3b6cb5b1ba0c682/src/main/java/fi/tkgwf/ruuvi/strategy/LimitingStrategy.java

I moved the db.store(measurement) part from the Main class into its own new PersistenceService, which allows me to test everything more easily by keeping the Main class clean by isolating the DBConnection and the strategy things out of it. So far I haven't changed the functionality^*) in these 15 commits, next I should finally start working on the actual new features. :smile: Any comments so far?

^(* OK, I lied: if a device sends data in two different protocols, it now gets filtered as one device. In the past every protocol had its own rate limiter. I suppose we can live with this change though, no?

[Scrin]

Quickly skimming through the changes, most of the changes look good so far, good work there 👍 I have some comments, suggestions and/or questions regarding some of the changes you have done, how would you prefer to hear about them? as comments on the commits where they were done or something else? I'll hopefully have some time this weened to review the changes in more detail.

Regarding the "now gets filtered as one device"; the original reason for treating every data format as "separate" was because some advanced tag firmwares may send different data formats, for example send temperature, pressure, etc once a minute but send all accelerometer readings 5 times a minute, or even send so much different data that it simply can't fit in one advertisement packet (and thus one data format). Though as no such firmwares and/or custom data formats exist yet (as far as I know), the change you did is okay if you feel it's beneficial. And of course that can always be changed back to the "old logic" if a need arises.

[ZeroOne3010]

Well, I could create a pull request already and you could comment on that? That should allow you to comment on all of the latest versions of all of the changed files at once. But if you have some very specific comments on some commits then by all means comment on those. I'm fine with any way, and I even just joined the Ruuvi Slack, so you can even find me there.

[Scrin]

Good point, maybe that's the most convenient way, so go ahead and create a PR with changes so far and I'll comment on that. I'll leave it open until all changes are done and the result looks good

[ZeroOne3010]

Great, and there you have it: pull request #12.