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

Always show data for sensors and handle sensor errors #63

Merged
merged 7 commits into from
Mar 4, 2019

Conversation

fungjj92
Copy link
Contributor

@fungjj92 fungjj92 commented Feb 27, 2019

Overview

The app will show the most up to date data, and we want that to be the case when sensor requests fail too. #20 set up initial data retrieval and this PR deepens that work by adding error handling.

The fall through process prefers in this order:

most recent real data of:

  • live sensor data as recent as exists
  • quarterly water quality data

else:

  • stale saved data
  • dummy default values

The dummy values I based off of current-ish values I was seeing from each sensor but that don't have any meaning really.

Connects #59

Demo

screen shot 2019-02-27 at 1 44 14 pm

Notes

I don't feel great about it, but in order to get data from the USGS APIs when they fail, we need to supply a starting date querystring, which is the last known date that the sensor has data. I added those dates to constants. They will always work, but become outdated over time insofar that we'll poll for more data than we need. I did not find a programatic and dynamic way to discover these dates.

I plan to rebase on #62 before merge

Testing Instructions

./scripts/server and see that the redux store has real data for all 3 sensors !

Please logically step through the pollSensor action and double check what I've done.

fungjj92 added 2 commits March 1, 2019 15:31
Sometimes, the USGS sensors fail to collect live data or the API call breaks. Fall back to using manual quarterly USGS data, prior saved data, or arbitrary default values in that order.

Refs #59
@fungjj92 fungjj92 force-pushed the jf/error-handle-sensor-requests branch from fe6c1c3 to 817367c Compare March 1, 2019 20:31
@fungjj92
Copy link
Contributor Author

fungjj92 commented Mar 1, 2019

Rebased on develop including #62, and tested. Appears fine 🤞

Copy link
Contributor

@caseycesari caseycesari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This appears to be working as we need it. Good job! I also have concerns about the fixed "last known data date", but I'm not sure how to get around that at the moment. Perhaps it's worth creating an issue with the notes, so that we can discuss alternative approaches in the future.

Most of my comments are about clean-up related stuff. Particularly, it looks like Prettier hasn't been run through the changes (details here about setting up your editor: https://prettier.io/docs/en/editors.html).

src/app/src/app.actions.js Outdated Show resolved Hide resolved
src/app/src/app.actions.js Outdated Show resolved Hide resolved
src/app/src/sensorUtils.js Show resolved Hide resolved
@fungjj92 fungjj92 force-pushed the jf/error-handle-sensor-requests branch from 817367c to 472cd33 Compare March 4, 2019 20:18
fungjj92 added 4 commits March 4, 2019 16:03
The live sensor API returns only the most recent reading taken if a time frame is not specified. We are seeing junk/no data since 12/2018 for both live sensors. Given a start date, the API sends back a list of only readings with real data since then, plus 1 additional row for the most recent sensor reading, if it is no data. Read this list in reverse order and check for no data values to grab the most recent, real, live sensor data.

Refs #59
This way, if sensor requests come back slowly or fail when the app initializes, the app will work fully albeit with dummy values.

Refs #59
Copy link
Contributor

@caseycesari caseycesari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revisions look good! Tested it with and without wi-fi, and things look right.

@caseycesari caseycesari assigned fungjj92 and unassigned caseycesari Mar 4, 2019
@fungjj92
Copy link
Contributor Author

fungjj92 commented Mar 4, 2019

Thanks for the reviews! Beat me to commenting.

Prettier is now set up the work on save on my editor, VS Code. I had installed several Prettier plugins last week, but ultimately I needed to npm install Prettier locally for the plugin(s) to work in the editor.

I created #63 to capture the discussion of date constants.

@fungjj92 fungjj92 merged commit e75295e into develop Mar 4, 2019
@fungjj92 fungjj92 deleted the jf/error-handle-sensor-requests branch March 4, 2019 22:01
fungjj92 added a commit that referenced this pull request Mar 6, 2019
Correct related constants and conditions
Also fix test data missing `timestamp` fields, which were added in #63

Refs #42
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.

2 participants