-
Notifications
You must be signed in to change notification settings - Fork 35
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
Sensor view #99
Sensor view #99
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great stuff.
I believe further discussion about some bigger design decisions is still to finalise between us. I already give some initial opinions. Also, some smaller stuff.
Question: from your earlier work (which you drew from here), the ability to add data to a chart is still to be applied right? Any other bigger items which are still not in this PR?
Indeed, I left out some major components of my earlier work. Just three, actually:
|
Maybe make issues for porting them over? At least the first one. |
I made a separate issue for intelligent chart updating as you suggested. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ask for moving some functions around.
Also, from our discussion : I believe JSON endpoints should already be hooked into the /api/2_0 path. That would make it easier to see the difference and it's future proof right away. I believe that is not too hard. I could try my hand on it if you agree.
I thought we expressly didn't want to move them there, and only note our intentions to do so later. Already moving them into our versioned user API brings potential user confusion (given project #3) and a premature maintenance burden. I'd like to write these as if they are already user API endpoints, but keep them as developer API endpoints until more mature. |
Maybe that was indeed the outcome of our discussion. But then I miss the clear documentation in In the call, we discussed the
|
Nice suggestion. I now moved the SensorAPI FlaskView into the API package in 65122e4 (and d0ee431, which I forgot to amend). This was the first time we registered a FlaskView instead of a Blueprint there, so please check my work. To test:
And the dev "version" of the our API remained undocumented after running |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The usage of Flask-Classful looks correct! I researched what the main advantage is over Flask.MethodView, and it seems the one thing to point out is the ability to create custom verbs, not just for REST methods.
From https://github.com/teracyhq/flask-classful/Readme:
"Can't I just use the base classes in flask.views?
Well, yes and no. While flask.views.MethodView does provide some of the functionality of flask_classful.FlaskView it doesn't quite complete the picture by supporting methods that aren't part of the typical CRUD operations for a given resource, or make it easy for me to override the route rules for particular view."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe I found a regression from earlier changes.
* Move code to data package Co-authored-by: F.N. Claessen <felix@seita.nl>
Co-authored-by: Nicolas Höning <iam@nicolashoening.de>
This PR adds a new UI view for individual Sensors, initially meant to shows Altair graphs of sensor data. The ability to graph sensor data is implemented as a new Sensor method, which is accessible via the API. This PR currently exposes all sensor attributes to the API, but we may choose to limit that to only a list of specific attributes to expose.
For now I propose that API functions relating to Sensors are not part of the official documentation, until the new data model is properly integrated.
Review instructions
example_data.csv contains, for example:
New endpoints:
Get interactive view on the sensor data
http://localhost:5000/sensors/66/
Get sensor attributes
Name: http://localhost:5000/sensors/66/name/
Timezone: http://localhost:5000/sensors/66/timezone/
Chart specification: http://localhost:5000/sensors/66/chart/
Chart data within some time range: http://localhost:5000/sensors/66/chart_data/?event_starts_after=2020-12-03T14:05:00+00:00&event_ends_before=2020-12-03T14:15:00+00:00
Get sensor chart as standalone html
All data: http://localhost:5000/sensors/66/chart/?include_data=true&as_html=true
Within some time range: http://localhost:5000/sensors/66/chart/?include_data=true&as_html=true&event_starts_after=2020-12-03T14:05:00+00:00&event_ends_before=2020-12-03T14:15:00+00:00