-
Notifications
You must be signed in to change notification settings - Fork 81
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
Update configuration.md #179
base: master
Are you sure you want to change the base?
Conversation
Changed examples according to the example config. Added link to the example config file
WalkthroughThe pull request updates the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
🧹 Nitpick comments (4)
companion/configuration.md (4)
10-11
: Format the URL as a proper Markdown linkThe bare URL should be formatted as a Markdown link for better readability and to follow Markdown best practices.
-You can find a working configuration file here: https://github.com/ESPresense/ESPresense-companion/blob/main/src/config.example.yaml +You can find a working configuration file [here](https://github.com/ESPresense/ESPresense-companion/blob/main/src/config.example.yaml).🧰 Tools
🪛 Markdownlint (0.37.0)
11-11: null
Bare URL used(MD034, no-bare-urls)
81-81
: Consider specifying measurement units in the commentThe bounds configuration looks good, but it would be helpful to specify the expected measurement units in the comment.
- bounds: [[0, 0, 0], [17.2, 18.5, 2.6]] # Centers your diagram. [[left, bottom, z], [right, top, z]] + bounds: [[0, 0, 0], [17.2, 18.5, 2.6]] # Centers your diagram. [[left, bottom, z], [right, top, z]] (in meters)
87-96
: Consider adding a shape description commentWhile the coordinates are correct and form a closed polygon, it would be helpful to add a comment describing the shape (e.g., L-shaped room, rectangular) to help users understand the example better.
# Rooms are members of one floor, so they go aligned to the bounds of their floor - name: Livingroom + # This example shows an L-shaped room points: - [6, 12] - [8.6, 12] - [8.6, 10] - [8, 9] - [6, 9] - [6, 12]
133-133
: Remove trailing punctuation from headingAccording to Markdown best practices, headings should not end with punctuation.
-## Help write this documentation! +## Help write this documentation🧰 Tools
🪛 Markdownlint (0.37.0)
133-133: Punctuation: '.'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
companion/configuration.md
(3 hunks)
🧰 Additional context used
🪛 Markdownlint (0.37.0)
companion/configuration.md
11-11: null
Bare URL used
(MD034, no-bare-urls)
133-133: Punctuation: '.'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
companion/configuration.md
Outdated
- name: Master | ||
point: [3.25, 11, 3.2] | ||
floors: ["first"] |
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.
Ensure floor ID consistency in examples
The node example references a floor ID "first", but the earlier floor configuration example uses "ground". This might confuse users.
- name: Master
point: [3.25, 11, 3.2]
- floors: ["first"]
+ floors: ["ground"]
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
- name: Master | |
point: [3.25, 11, 3.2] | |
floors: ["first"] | |
- name: Master | |
point: [3.25, 11, 3.2] | |
floors: ["ground"] |
Link formated as Markup-Link Units meters added to yaml examples
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.
Actionable comments posted: 0
♻️ Duplicate comments (1)
companion/configuration.md (1)
107-109
:⚠️ Potential issueEnsure floor ID consistency in examples
The node example references a floor ID "first", but the earlier floor configuration example uses "ground". This might confuse users.
- name: Master point: [3.25, 11, 3.2] - floors: ["first"] + floors: ["ground"]
🧹 Nitpick comments (3)
companion/configuration.md (3)
10-11
: Consider using a relative link for better maintainabilityThe link to the example configuration file should use a relative path to ensure it remains valid if the repository is renamed or forked.
-You can find a working configuration file [here](https://github.com/ESPresense/ESPresense-companion/blob/main/src/config.example.yaml) +You can find a working configuration file [here](../../../ESPresense-companion/blob/main/src/config.example.yaml)
88-98
: Consider adding a note about closed polygonsThe example correctly shows that the first and last points should match to form a closed polygon. This could be explicitly mentioned in the documentation.
# Rooms are members of one floor, so they go aligned to the bounds of their floor # Units are always meters +# Note: The first and last points should be identical to form a closed polygon
136-136
: Remove trailing period from headingAccording to markdown style guidelines, headings should not end with punctuation.
-## Help write this documentation!. +## Help write this documentation!🧰 Tools
🪛 Markdownlint (0.37.0)
136-136: Punctuation: '.'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
companion/configuration.md
(3 hunks)
🧰 Additional context used
🪛 Markdownlint (0.37.0)
companion/configuration.md
136-136: Punctuation: '.'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
🔇 Additional comments (1)
companion/configuration.md (1)
78-82
: LGTM! Clear and well-documented bounds format
The new nested array format for bounds is more explicit and the comment clearly explains the structure.
Ground floor instead of first floor to keep the examples constant
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.
Actionable comments posted: 0
🧹 Nitpick comments (3)
companion/configuration.md (3)
10-11
: Consider using a more stable reference formatWhile adding a reference to the example configuration is helpful, raw GitHub branch links can break if the repository structure changes. Consider:
- Using a tagged release or commit hash instead of branch reference
- Adding a brief note about potential differences between documentation and latest example
-You can find a working configuration file [here](https://github.com/ESPresense/ESPresense-companion/blob/main/src/config.example.yaml) +You can find a working configuration file [here](https://github.com/ESPresense/ESPresense-companion/blob/v1.0.0/src/config.example.yaml) (based on version 1.0.0)
88-98
: Clarify the dimensionality of room pointsWhile floor bounds use 3D coordinates, room points are defined in 2D. This should be explicitly mentioned to avoid confusion.
# Rooms are members of one floor, so they go aligned to the bounds of their floor # Units are always meters +# Points are defined in 2D space [x,y] as rooms are mapped to their floor's z-coordinate - name: Livingroom
136-136
: Remove trailing punctuation from headingAccording to markdown best practices, headings should not end with punctuation.
-## Help write this documentation! +## Help write this documentation🧰 Tools
🪛 Markdownlint (0.37.0)
136-136: Punctuation: '.'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
companion/configuration.md
(3 hunks)
🧰 Additional context used
🪛 Markdownlint (0.37.0)
companion/configuration.md
136-136: Punctuation: '.'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
🔇 Additional comments (2)
companion/configuration.md (2)
78-82
: LGTM! Clear improvement in coordinate representation
The updated bounds format with nested arrays and explicit coordinate naming makes the 3D space configuration more intuitive.
104-109
: LGTM! Floor ID consistency maintained
The node configuration now correctly references the "ground" floor ID, maintaining consistency with the floor configuration example.
Pulled in changes from ESPresense#179 and made a few formatting fixes
* Update configuration.md Pulled in missing info from example config and updated explanations * Update configuration.md coderabbitai fixes * Update configuration.md Pulled in changes from #179 and made a few formatting fixes
Changed examples according to the example config file.
Added a link to the example config file
The examples in the configuration documentation file differ from the sample configuration file. The syntax of the example file works perfectly, the syntax of the documentation does not.
Summary by CodeRabbit
New Features
Documentation