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 for newer devices #34

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Fix for newer devices #34

wants to merge 7 commits into from

Conversation

Xenomes
Copy link

@Xenomes Xenomes commented Oct 17, 2022

It is not the best coding, but it works 😅

Change encoding/decoding and some clean up.
Add instruction for running as a service.
double line
remove log report
@jodewee
Copy link

jodewee commented Oct 18, 2022

Hi, how do i get this in docker?

@@ -17,6 +17,7 @@
},
"dependencies": {
"dotenv": "^8.1.0",
"mcrypt": "^0.1.17",
Copy link
Owner

Choose a reason for hiding this comment

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

As I see mcrypt package has been deprecated and not maintained anymore. The author suggests using cryptian instead

Copy link
Author

@Xenomes Xenomes Oct 23, 2022

Choose a reason for hiding this comment

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

mcrypt was the only decoder showing the strange characters after } in the json returning from the ac device and cryptian requires V19 and the use base is complete different.

@@ -1,7 +1,7 @@
FROM node:alpine
FROM node:10
Copy link
Owner

Choose a reason for hiding this comment

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

NodeJS v10 is pretty old and not supported anymore. Are there any reasons we can't use the latest LTS version as it was before?

Copy link
Author

@Xenomes Xenomes Oct 23, 2022

Choose a reason for hiding this comment

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

Thats my bad, mcrypt needs the full version of node and not specially v10. Used V10 because that was the default on my test system Debian Buster the OS for Domoticz stable.

@find-a-good-nickname
Copy link

Tried your fix (edited connection.js, device_manager.js, encryptor.js, package.json, package-lock.json files and index.js with the IP of the AC) before npm install, but then this Mcrypt is still missing:

pi@raspberrypi:~/ewpe-smart-mqtt $ npm start

> [email protected] start
> node index.js

2022-10-27T20:29:01.797Z [info]: Trying to connect to MQTT server mqtt://127.0.0.1 ...
2022-10-27T20:29:01.955Z [info]: Successfully connected to MQTT server
2022-10-27T20:29:01.980Z [info]: Socket server is listening on 0.0.0.0:46861
2022-10-27T20:29:01.981Z [info]: Scanning network 192.168.10.71 for available devices...
/home/pi/ewpe-smart-mqtt/app/encryptor.js:15
        const aesEcb = MCrypt('rijndael-128', 'ecb')
                       ^

ReferenceError: MCrypt is not defined
    at decrypt (/home/pi/ewpe-smart-mqtt/app/encryptor.js:15:24)
    at Connection.handleResponse (/home/pi/ewpe-smart-mqtt/app/connection.js:110:26)
    at Socket.emit (node:events:390:28)
    at UDP.onMessage [as onmessage] (node:dgram:939:8)

I tried then to find a solution on the net to install mcrypt on to a RPi4, but failed, so any help is welcome! Thanks!

@Xenomes
Copy link
Author

Xenomes commented Oct 28, 2022

Hi you run npm install then the module mcrypt installed. If not you package.json is missing mcrypt. You can run npm install mcrypt.

@find-a-good-nickname
Copy link

find-a-good-nickname commented Oct 28, 2022

Tried to install mcrypt as you said (nothing happened), but is still missing:

pi@raspberrypi:~/ewpe-smart-mqtt $ npm install mcrypt

up to date, audited 79 packages in 2s

5 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities
pi@raspberrypi:~/ewpe-smart-mqtt $ npm start

> [email protected] start
> node index.js

2022-10-28T07:04:45.650Z [info]: Trying to connect to MQTT server mqtt://127.0.0.1 ...
2022-10-28T07:04:45.807Z [info]: Successfully connected to MQTT server
2022-10-28T07:04:45.829Z [info]: Socket server is listening on 0.0.0.0:46042
2022-10-28T07:04:45.831Z [info]: Scanning network 192.168.10.71 for available devices...
/home/pi/ewpe-smart-mqtt/app/encryptor.js:15
        const aesEcb = MCrypt('rijndael-128', 'ecb')
                       ^

ReferenceError: MCrypt is not defined
    at decrypt (/home/pi/ewpe-smart-mqtt/app/encryptor.js:15:24)
    at Connection.handleResponse (/home/pi/ewpe-smart-mqtt/app/connection.js:110:26)
    at Socket.emit (node:events:390:28)
    at UDP.onMessage [as onmessage] (node:dgram:939:8)

Node version: 16.13.1, Npm version: 8.19.2

@Xenomes
Copy link
Author

Xenomes commented Oct 28, 2022

That is strange, can you list your or check the first line of encryptor.js file?
My node is 19.0.0 and NPM version is the same, I had it is running in Node 10 in docker.

@find-a-good-nickname
Copy link

First line in /app/encryptor.js is:

const crypto = require('mcrypt').MCrypt;

@Xenomes
Copy link
Author

Xenomes commented Oct 28, 2022

This line is wrong. Please check the file or clone my github and chance branche to fix.

@find-a-good-nickname
Copy link

Oh, sorry, my fault!
Finally I edited the line to

const MCrypt = require('mcrypt').MCrypt;

...and it WORKS perfectly!

Thanks Stas and Xenomes, thanks and thanks again!

@Xenomes
Copy link
Author

Xenomes commented Oct 29, 2022

Good to hear it is working!

@johndoe3github
Copy link

Many thanks for the fix and pull request! But I think your fix @ https://codeload.github.com/Xenomes/ewpe-smart-mqtt/zip/refs/heads/fix is way to hidden! It took me 3 days to find it and implement it to find our new Sinclair besides our older Gree at Domoticz with Mosquitto running npm start only to give error: internal/crypto/cipher.js:164 const ret = this._handle.final();

@Xenomes
Copy link
Author

Xenomes commented Feb 18, 2023

Thanks, it is not the best fix, but it works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants