Filter listener sensor graphs by value and date range.

[paul121]

This adds a farm_sensor_listener_data_graphs_form which renders graphs of sensor data with a fieldset of filters for value and date range. This is only a minor step towards a solution for #227, but seems likely that this is the most we will accomplish in farmOS 1.x. While there is greater support for graphing data with views in D8, it would be quite a bit of work to get it working in D7.

I chose to make the graphs load a default date range (the past 7 days) rather than a default limit of 100 data points. Overall this seems more intuitive for users. The downside is that sensors which have not posted data within the past week, but have older values, will not display graphs of data by default. Open to ideas on this! Perhaps we could add a fallback condition to display the past X data points if the date range fails? Also, because this is a form, I believe that those loading the form could pre-load the form state with default filter values too.

To make the date range work I had to make a change to the farm_sensor_listener_data helper function to allow disabling the limit (see notes on this commit: 1d95005). This seems to be the best solution that doesn't require changing the function's default values which might affect other current uses (especially those returning data via API).

[paul121]

Data filtered by date range:

Message that a value has no data for the specified date range:

[mstenta]

This is great @paul121 !

This is only a minor step towards a solution for #227, but seems likely that this is the most we will accomplish in farmOS 1.x. While there is greater support for graphing data with views in D8, it would be quite a bit of work to get it working in D7.

👍 Makes sense to me.

I chose to make the graphs load a default date range (the past 7 days) rather than a default limit of 100 data points. Overall this seems more intuitive for users. The downside is that sensors which have not posted data within the past week, but have older values, will not display graphs of data by default.

This is the one thing that's stopping me from merging this as-is. When I first added the graphs, I went back and forth between loading the past "X days of data" vs the past "X data points", and decided to go with "X data points" for this exact reason. I want old sensors to show their "last" data in the graph, even if it was not recent.

Perhaps we could add a fallback condition to display the past X data points if the date range fails? Also, because this is a form, I believe that those loading the form could pre-load the form state with default filter values too.

I kinda like the latter idea there... try to determine when the last data point was and set the default start/end dates of the filters accordingly. This would work for old and current sensors, I think.

---

One small thing to change: I like the "No data in this time range" message. But: we try to avoid using Bootstrap styles outside of farm_theme - so that the farmOS modules are theme agnostic and all styles/display logic are maintained centrally in farm_theme.

An easy workaround would be: instead of building that message as markup that appears in the graphs fieldset, add it to the top of the page via drupal_set_message(), which will get those styles automatically from the theme. It's a bit worse because it's not in the fieldset, but that's OK I think. Or... just display the text without styles.

Also minor minor nit: only capitalize the first word in "Sensor Graphs".

[paul121]

@mstenta so I just realized it would be helpful to provide a list of sensor value names via the API (useful for viewing data in Field Kit) at farm/sensor/listener/values/{public_key}. In the process of adding that, I realized farm_sensor_listener_values could return the first and last timestamps of data saved for each respective value. This also makes it easy to display the latest week of data in the sensor graphs page.

What do ya think? That function & API path could be used to return additional info in the future, too. Perhaps a translated label of the sensor value?

Made the small changes regarding boostrap styles & text capitalization. Makes sense on bootstrap - lets just remove the class for now.

[paul121]

Oh maybe this should be farm/sensor/listener/{public_key}/values ?

[paul121]

Here is an example of data returned from the farm/sensor/listener/values endpoint:

{
  "Temperature": {
    "first": "1596355200",
    "last": "1597129200"
  },
  "Moisture": {
    "first": "1596355200",
    "last": "1597129200"
  },
  "Other value": {
    "first": "1596355200",
    "last": "1597129200"
  }
}

[mstenta]

Hi @paul121 - looking over this again now with the hopes of merging before 7.x-1.5...

Noticed a small issue:

  // Limit the results.
  // Don't apply a range if both offset and limit are NULL.
  if (!empty($offset) && !empty($limit)) {
    $query->range($offset, $limit);
  }

The default value of the $offset parameter is 0, which means that $query->range() won't be added when it should. We could fix that by changing both !empty() to !is_null() so that it explicitly checks for NULL values. And with that in mind, I think we can simplify by only checking that $limit is null, I don't think we also need to check $offset. I will go ahead and make that change.

[mstenta]

On second thought - rather than checking !is_null() I'll leave it as !empty() but ONLY check the $limit parameter. So, in order to load all data, you just need to set $limit to 0. Make sense?

I will also update the call to this function where you used NULL, NULL in the function parameters to just use 0.

[mstenta]

(I also rebased onto latest 7.x-1.x and force pushed.)

[mstenta]

I'm looking through sensors in the Wolfe's Neck database. Noticing some strange "empty data" behavior - not working as expected I think...

Here is a sensor that never had any data pushed to it: http://localhost/farm/sensor/high-roller-humidity

I think we should omit the "Sensor graphs" fieldset entirely in that scenario.

Here are two sensors that have SOME data, but the graphs are not showing and the empty text is:

http://localhost/farm/sensor/dons-co-deployment-sensor-1

http://localhost/farm/sensor/dwbgatewaya

Not sure why those aren't showing graphs.

[mstenta]

What do ya think? That function & API path could be used to return additional info in the future, too. Perhaps a translated label of the sensor value?

I like it! Having a place for high-level meta data for each sensor makes a lot of sense.

Oh maybe this should be farm/sensor/listener/{public_key}/values ?

So actually... what if we just add an additional GET parameter option to the existing page callback? And when it is present it shows the meta/summary info instead of the values.

Perhaps something like: farm/sensor/listener/{public_key}?summary=1

I feel like the distinction between farm/sensor/listener/{public_key}/values vs farm/sensor/listener/{public_key} would not be clear ("values" implies you'll be getting values back, but it's actually summary info, not values).

Also using the existing page callback means less duplicated code (eg: the logic for checking the private key, adding the Access-Control-Allow-Origin header, etc).

And with all the above in mind, what do you think about renaming the farm_sensor_listener_values() to farm_sensor_listener_values_info() to make it more clear that it's meta info, not values, that will be returned?

I have this sketched up... will push commits shortly for your review...

[mstenta]

Chatted with @paul121 and decided a /summary argument on the end of the path would be better than a ?summary=1 GET parameter. Pushed a commit to change that: 7ed5045

[mstenta]

So I think we just need to fix the "empty" text issues (described above: #337 (comment)) - and then this is ready to merge!

[paul121]

Thanks @mstenta! Just added two commits to fix those issues. Data wasn't displaying because the timestamp was being rounded to 12am that day in order to fit the datetime input. I went ahead and added the hour + minutes + seconds fields and it fixes the problem.

[paul121]

Ah - this introduced a little bug. A check using !empty() didn't allow passing limit=0 via the rest API.

[mstenta]

Talked with @paul121 yesterday and added a few more commits:

• Use isset() for all param and arg checks instead of !empty() and !is_null() because it's more explicit, and fixes "undefined index" issue when the param isn't included.
• We discovered how easy it is to use up all of PHP's available memory by requesting an unlimited amount of data from a sensor (100,000 data points worked, albeit slowly. 1,000,000 data points breaks.) So, now we're enforcing a 100k data point cap when reading data via the API. If ?limit=0 is used, it will automatically limit to 100k. If ?limit=100001 is requested, a 422 response code will be returned, with no data. So in order to get all data, multiple requests using limit and offset will be required.
• Added count to the /summary info so that API users can see exactly how many data points exist for a given name. This will help in paging through result sets greater than 100k.

[mstenta]

Also documented these changes in the farmOS.org repo's 7.x-1.5 branch:

• farmOS/farmOS.org@93e8b9d
• farmOS/farmOS.org@103dd0a

[mstenta]

Rebased and merged! Thanks again @paul121 !