-
-
Notifications
You must be signed in to change notification settings - Fork 56
Parse RatOS dialect version when used #91
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
base: development
Are you sure you want to change the base?
Conversation
WalkthroughThe validation logic for RatOS G-code flavour in the Actions module was updated to check the Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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.
Pull Request Overview
This PR fixes an oversight in the version check for the RatOS dialect by comparing against the correct property.
- Fixes the version comparison to use ratosDialectVersion instead of generatorVersion.
- Updates the error message to reflect the correct version field.
Comments suppressed due to low confidence (2)
src/server/gcode-processor/Actions.ts:123
- The version check now uses ratosDialectVersion as intended, but please confirm that all related references and documentation align with this change.
if (semver.neq('0.1', gcodeInfo.ratosDialectVersion)) {
src/server/gcode-processor/Actions.ts:125
- Ensure that the updated error message accurately communicates that the check is for the RatOS dialect version and that any associated documentation is updated accordingly.
`Only version 0.1 of the RatOS G-code dialect is supported. Version ${gcodeInfo.ratosDialectVersion} is not supported.`,
Note that due to Rat-OS/RatOS-configurator#91 the tool version needs to be set to 0.1, otherwise RatOS will refuse to print the G-Code unless the user overrides it in their Klipper config (unlikely to do so since it's undocumented)
No, it hasn't been defined yet. When writing the new postprocessor, I added it as a concept to support custom GCode generators, for example programmatically-generated gcode (which I do for my own projects). For IDEX prints, it should be easier to emit the post-processed style of Anyhow, your change looks good to me, even though no user should be hitting that code path at the moment. Well spotted! |
Thanks! I spotted this as I'm trying to add support for RatOS to "Ellis' Pressure Advance / Linear Advance Calibration Tool" FYI AndrewEllis93/Pressure_Linear_Advance_Tool#48 I'm still hitting some issues though |
Note that due to Rat-OS/RatOS-configurator#91 the tool version needs to be set to 0.1, otherwise RatOS will refuse to print the G-Code unless the user overrides it in their Klipper config (unlikely to do so since it's undocumented)
|
@depau the established escape hatch for stuff like Ellis gcode is covered here. tl;dr: run However, this might still be only on the |
The header comment says:
RatOS-configurator/src/server/gcode-processor/Actions.ts
Lines 50 to 56 in 420f1a7
However there must be an oversight since
generatorVersionis being compared anyway. This fixes it.By the way, are there any specs on what exactly qualifies as "RatOS dialect"?
Summary by CodeRabbit