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

DOCS-821: Add Power Sensor Component #1766

Merged
merged 21 commits into from
Sep 18, 2023
Merged

Conversation

skyleilani
Copy link
Contributor

Add power sensor component supporting INA219, INA226, and renogy

@viambot viambot added the safe to build This pull request is marked safe to build from a trusted zone label Sep 7, 2023
@npentrel npentrel force-pushed the DOCS-821-Add-Power-Sensor branch from 81f17a4 to 266e92f Compare September 10, 2023 17:01
Copy link
Contributor

@andf-viam andf-viam left a comment

Choose a reason for hiding this comment

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

This is fantastic! Looking good overall, just some small language nits and minor corrections. Great job!!

docs/components/power-sensor/_index.md Outdated Show resolved Hide resolved
docs/components/power-sensor/_index.md Outdated Show resolved Hide resolved
docs/components/power-sensor/_index.md Outdated Show resolved Hide resolved
docs/components/power-sensor/_index.md Outdated Show resolved Hide resolved
docs/components/power-sensor/_index.md Outdated Show resolved Hide resolved
docs/components/power-sensor/renogy.md Outdated Show resolved Hide resolved
static/include/components/apis/power-sensor.md Outdated Show resolved Hide resolved
docs/components/power-sensor/ina226.md Outdated Show resolved Hide resolved
docs/components/power-sensor/ina219.md Outdated Show resolved Hide resolved
docs/components/power-sensor/_index.md Outdated Show resolved Hide resolved
@viambot
Copy link
Member

viambot commented Sep 12, 2023

Overall readability score: 54.7 (🟢 +0.08)

File Readability
_index.md 59.33 (-)
fake.md 49.32 (-)
ina219.md 62.38 (-)
ina226.md 59.69 (-)
renogy.md 44.2 (-)
_index.md 49.56 (🟢 +0)
foam-dart-launcher.md 59.54 (🟢 +0)
power-sensor.md 85.77 (-)
show-secret.md 43.23 (🟢 +0)
View detailed metrics

🟢 - Shows an increase in readability
🔴 - Shows a decrease in readability

File Readability FRE GF ARI CLI DCRS
_index.md 59.33 40.24 8.05 14.2 13.45 6.41
  - - - - - -
fake.md 49.32 45.35 11.56 14.5 11.83 8.49
  - - - - - -
ina219.md 62.38 58.08 9.31 11.7 10.48 8.14
  - - - - - -
ina226.md 59.69 56.35 9.99 12.2 10.66 8.22
  - - - - - -
renogy.md 44.2 42.82 13.1 15.6 12.53 8.33
  - - - - - -
_index.md 49.56 37.4 10.55 15.2 13.45 7.52
  🟢 +0 🟢 +0 🟢 +0 🟢 +0 🟢 +0 🟢 +0
foam-dart-launcher.md 59.54 56.89 11.12 14.4 11.09 6.51
  🟢 +0 🟢 +0 🟢 +0 🟢 +0 🟢 +0 🟢 +0
power-sensor.md 85.77 67.04 7.19 6 6.54 6.76
  - - - - - -
show-secret.md 43.23 36.49 11.06 15.3 13.8 8.97
  🟢 +0 🟢 +0 🟢 +0 🟢 +0 🟢 +0 🟢 +0

Averages:

  Readability FRE GF ARI CLI DCRS
Average 54.7 46.15 10.87 13.32 11.82 7.77
  🟢 +0.08 🟢 +0.05 🟢 +0.02 🟢 +0.01 🟢 +0.01 🟢 +0
View metric targets
Metric Range Ideal score
Flesch Reading Ease 100 (very easy read) to 0 (extremely difficult read) 60
Gunning Fog 6 (very easy read) to 17 (extremely difficult read) 8 or less
Auto. Read. Index 6 (very easy read) to 14 (extremely difficult read) 8 or less
Coleman Liau Index 6 (very easy read) to 17 (extremely difficult read) 8 or less
Dale-Chall Readability 4.9 (very easy read) to 9.9 (extremely difficult read) 6.9 or less

Copy link
Contributor

@andf-viam andf-viam left a comment

Choose a reason for hiding this comment

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

LGTM!! Just three little language tweaks, then good to go!

docs/components/power-sensor/renogy.md Show resolved Hide resolved
docs/components/power-sensor/renogy.md Outdated Show resolved Hide resolved
docs/tutorials/projects/foam-dart-launcher.md Show resolved Hide resolved
| `serial_path` | string | **Required** | The full filesystem path to the serial device, starting with <file>/dev/</file>. With your serial device connected, you can run `sudo dmesg \| grep tty` to show relevant device connection log messages, and then match the returned device name, such as `ttyS0`, to its device file, such as <file>/dev/ttyS0</file>. If you omit this attribute, Viam will attempt to automatically detect the path.<br>Default: `/dev/serial0` |
| `serial_baud_rate` | int | **Required** | The baud rate to use for serial communications. <br> Default: `9600` |
| `modbus_id` | int | **Required** | Controller MODBUS address. <br> Default: `1` |
| `serial_path` | string | Optional | The full filesystem path to the serial device, starting with <file>/dev/</file>. With your serial device connected, you can run `sudo dmesg \| grep tty` to show relevant device connection log messages, and then match the returned device name, such as `ttyS0`, to its device file, such as <file>/dev/ttyS0</file>. If you omit this attribute, Viam will attempt to automatically detect the path.<br>Default: `/dev/serial0` |
Copy link
Member

Choose a reason for hiding this comment

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

might also be helpful to include a line to look in /dev/serial/by-path to find the serial path? see the roboclaw motor doc for an example.

| `serial_path` | string | **Required** | The full filesystem path to the serial device, starting with <file>/dev/</file>. With your serial device connected, you can run `sudo dmesg \| grep tty` to show relevant device connection log messages, and then match the returned device name, such as `ttyS0`, to its device file, such as <file>/dev/ttyS0</file>. If you omit this attribute, Viam will attempt to automatically detect the path.<br>Default: `/dev/serial0` |
| `serial_baud_rate` | int | **Required** | The baud rate to use for serial communications. <br> Default: `9600` |
| `modbus_id` | int | **Required** | Controller MODBUS address. <br> Default: `1` |
| `serial_path` | string | Optional | The full filesystem path to the serial device, starting with <file>/dev/</file>. With your serial device connected, you can run `sudo dmesg \| grep tty` to show relevant device connection log messages, and then match the returned device name, such as `ttyS0`, to its device file, such as <file>/dev/ttyS0</file>. If you omit this attribute, Viam will attempt to automatically detect the path.<br>Default: `/dev/serial0` |
Copy link
Member

Choose a reason for hiding this comment

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

it should start with just/dev/<file> so can remove the first <file>

icon: "/icons/components/sensor.svg"
images: ["/icons/components/sensor.svg"]
# SME: #team-bucket
---

Configure a `renogy` sensor to integrate a [Renogy battery temperature sensor](https://www.amazon.com/Renogy-Battery-Temperature-Sensor-Controllers/dp/B07WMMJFWY) into your robot:
Configure a `renogy` sensor to integrate a [Renogy battery temperature sensor](https://www.renogy.com/battery-temperature-sensor-for-renogy-solar-charge-controllers/) into your robot:
Copy link
Member

Choose a reason for hiding this comment

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

this is not an accurate link to the device, can you replace with this?. and replacing the link name to Renogy Charge Controller would be more accurate.

type: "docs"
description: "Configure a renogy model sensor."
tags: ["sensor", "components"]
description: "Configure a renogy model power sensor to return battery temperature."
Copy link
Member

Choose a reason for hiding this comment

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

Instead of return battery temp this now returns battery voltage and load current/power, along with other readings.

weight: 10
draft: false
type: "docs"
description: "Configure an INA219 model power sensor to return voltage and current readings."
Copy link
Member

Choose a reason for hiding this comment

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

optional but want to also include in the description that there is a power reading

"model": "INA219",
"attributes": {
"board": "<your-board-name>",
"i2c_bus": "<your-i2c-bus-name-on-board>"
Copy link
Member

Choose a reason for hiding this comment

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

looks like the template didn't get updated to the new attributes


| Attribute | Type | Inclusion | Description |
| --------- | -----| --------- | ----------- |
| `i2c_bus` | integer | **Required** | The `name` of the [I<sup>2</sup>C bus](/components/board/#i2cs) that the sensor is connected to. |
Copy link
Member

Choose a reason for hiding this comment

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

I think the description can be changed to number instead of name since it was changed to an integer value.

docs/components/power-sensor/ina226.md Show resolved Hide resolved
docs/components/power-sensor/ina219.md Outdated Show resolved Hide resolved
@viambot
Copy link
Member

viambot commented Sep 18, 2023

You can view a rendered version of the docs from this PR at https://docs-test.viam.dev/d9d5d91601a11a6e5847d83ae82cbcf65d17759c/public

@skyleilani skyleilani merged commit 28378ae into main Sep 18, 2023
8 checks passed
@skyleilani skyleilani deleted the DOCS-821-Add-Power-Sensor branch September 18, 2023 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
safe to build This pull request is marked safe to build from a trusted zone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants