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

fix(vendor.roborock): Fix S4 detection logic #729

Merged
merged 1 commit into from
Mar 3, 2021

Conversation

cmccambridge
Copy link
Contributor

Type of change

Type A:

  • Bug fix (non-breaking change which fixes an issue)
  • Refactor/Code Cleanup
  • Docs
  • Capability implementation for existing core capability
  • New robot implementation

Type B:

  • New capability
  • New core feature

Description (Type A)

Detection logic for the Roborock S4 / T4 was incorrect due to a typo. Fixing it :-)
Didn't file an issue for this, though I can if you want to track. Noticed it as latest dustbuilder doesn't load Valetudo successfully with this error:

root@rockrobo:~# VALETUDO_CONFIG_PATH=/mnt/data/valetudo_config.json /usr/local/bin/valetudo
[2021-03-03T17:29:38.987Z] [INFO] Loading configuration file: /mnt/data/valetudo_config.json
[2021-03-03T17:29:39.013Z] [INFO] Set Logfile to /tmp/valetudo.log
[2021-03-03T17:29:39.029Z] [ERROR] Error while initializing robot implementation. Shutting down  Error: Couldn't find a suitable ValetudoRobot implementation.
    at Function.autodetectRobotImplementation (/snapshot/Valetudo/lib/core/ValetudoRobotFactory.js:52:19)
    at Function.getRobotImplementation (/snapshot/Valetudo/lib/core/ValetudoRobotFactory.js:16:45)
    at new Valetudo (/snapshot/Valetudo/lib/Valetudo.js:28:62)
    at Object.<anonymous> (/snapshot/Valetudo/index.js:5:16)
    at Module._compile (internal/modules/cjs/loader.js:1198:30)
    at Module._compile (pkg/prelude/bootstrap.js:1327:32)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1218:10)
    at Module.load (internal/modules/cjs/loader.js:1047:32)
    at Function.Module._load (internal/modules/cjs/loader.js:935:14)
    at Function.Module.runMain (pkg/prelude/bootstrap.js:1375:12)

@cmccambridge
Copy link
Contributor Author

@Hypfer want to be super clear: I haven't tested this yet as I'm still working on build process locally here... but the change looks so clear from the source that I wanted to send it your way for any initial feedback, and a read on whether it's so clear that we can simply merge the fix :)

@Hypfer Hypfer merged commit 75974a5 into Hypfer:master Mar 3, 2021
@cmccambridge
Copy link
Contributor Author

Belated follow up, but just got the build done end to end and tested on my S4, it does indeed work for the S4 😁

Thanks!

@cmccambridge cmccambridge deleted the s4-detection branch March 3, 2021 18:47
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants