Add support for system values reporting #33
Conversation
inorbit_edge/robot.py
Outdated
| @@ -829,6 +831,35 @@ def set_pairs(key): | |||
|
|
|||
| self.publish_protobuf(MQTT_SUBTOPIC_CUSTOM_DATA, msg) | |||
|
|
|||
| def publish_system_stats(self, system_values): | |||
There was a problem hiding this comment.
I suggest changing this method's signature to publish_system_stats(self, cpu_load_percentage=None, ram_usage_percentage=None, hdd_usage_percentage=None, total_tx=None, total_rx=None, ts=None, elapsed_seconds=None) This way the SDK users can leverage IDE autocomplete features and minimize the chances of typos in the field names.
There was a problem hiding this comment.
nice suggestion, I agree :)
inorbit_edge/robot.py
Outdated
| msg.total_tx = total_tx | ||
| msg.total_rx = total_rx | ||
| msg.timestamp = ts if ts else int(time.time() * 1000) | ||
| msg.elapsed_seconds = elapsed_seconds |
There was a problem hiding this comment.
What happens if elapsed_seconds is not provided? If it's required we should either remove the default None value or calculate it using the previous system stat message ts
There was a problem hiding this comment.
if elapsed_seconds is not provided, the platform won't calculate the Network rate, since it uses a delta, I think calculating using the previous message could work
There was a problem hiding this comment.
Network rate just using last seconds won't work, since we need to calculate Deltas, for now, I'll leave to the user the possibility of implementing that, since calculating deltas etc will be a particular implementation that could not satisfy how the user wants this to work
|
As discussed with @leandropineda , we'll leave the network implementation for later, ready for a re-review |
Your checklist for this pull request
🚨Please review the guidelines for contributing to this repository.
Description
Adds support for systems stats reporting.
Now it is possible to report CPU load, RAM Usage and HDD Usage.
This change allows the Edge SDK to report system values, which can be seen in the vitals widget.