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

v2.1.0: Token read/write // refact code #382

Merged
merged 13 commits into from
Jun 22, 2024
Merged

v2.1.0: Token read/write // refact code #382

merged 13 commits into from
Jun 22, 2024

Conversation

bugsounet
Copy link
Contributor

@bugsounet bugsounet commented Jun 21, 2024

Change Log:

  • netatmo.js:

    • Review start rules and (please) don't use self... use this
    • start() send now config to helper.js and use it (without self!)
    • delete not needed payload on socket notification send (already sended on init)
    • delete error 403 loop !!!
  • helper.js:

    • make init function for init all value with this
    • check if all needed value are set in config (clientId, clientSecret, refresh_token)
    • Create a token.json file when Authenticate done (with new credentials)
    • Rules on first start: use refresh_token of config for autenticate
    • Next start/restart: use token.json file for authenticate
    • use new value for better management: this.clientId, this.clientSecret, this.refreshToken
    • add rules for refresh token
    • Set refresh token to 1 min before netatmo expire token response
  • Delete Deps:

    • @ungap/url-search-params : not needed with node > 18
  • Add eslint rules:

    • eslint.config.js from my common modules
  • Add Deps:

    • @eslint/js
    • @stylistic/eslint-plugin
  • package.json

    • review validate:js script
    • review fix:js script
  • lint (with eslint.config.js)

    • node_helper.js
    • helper.js
    • netatmo.js
    • helper.test.js is ignored (can't works now but i keep it... up to @CFenner to clean it)
  • workflows:

    • delete helper.test.js / .env.template : can't works now with new netatmo API Rules
    • update validation rules
    • Fix: renovate.json (for validate:json)
  • Add token.json to .gitignore

  • Update README.md

    • delete: cd netatmo && npm ci --production --ignore-scripts --> is now not needed
    • No special dependencies and no others commands are now needed!

@bugsounet
Copy link
Contributor Author

Your codes rules are really weird ;)

up to you to finish ;)

@bugsounet
Copy link
Contributor Author

I will see that add auto-refresh token
It's not added actually

@Lusbueb69
Copy link

Lusbueb69 commented Jun 21, 2024

maybe solve #380

I can't test better actually because i have no station

I just apply read/write token rules like my MMM-NetatmoThermostat module

@CFenner can you test it ?

what is the right way to test your code adjustments? If I make the adjustments manually and restart MM, then nothing is displayed anymore. If I remove your adjustments and restart MM, then everything works fine.

Is it not enough if I just adjust helper.js, do I need additional commands like cd netatmo && npm ci --production --ignore-scripts?

@bugsounet bugsounet marked this pull request as draft June 22, 2024 05:12
@bugsounet
Copy link
Contributor Author

bugsounet commented Jun 22, 2024

Right, I rewrite many thing...

Now result is on first start:

[2024-06-22 10:01:23.027] [LOG]   Connecting socket for: netatmo 
[2024-06-22 10:01:23.028] [LOG]   Netatmo helper started ... 
[2024-06-22 10:01:23.028] [LOG]   Sockets connected & modules started ... 
[2024-06-22 10:01:23.138] [LOG]   Launching application. 
[2024-06-22 10:01:24.173] [LOG]   Netatmo: using refresh_token from config 
[2024-06-22 10:01:24.173] [LOG]   Netatmo: Initialized 
[2024-06-22 10:01:24.498] [LOG]   Netatmo: Authenticated 
[2024-06-22 10:01:24.498] [LOG]   Netatmo: token.json was written successfully 
[2024-06-22 10:01:24.501] [LOG]   Netatmo: New Token Expire Saturday, June 22, 2024 1:01 PM 

on restart:

[2024-06-22 10:03:01.312] [LOG]   Connecting socket for: netatmo 
[2024-06-22 10:03:01.312] [LOG]   Netatmo helper started ... 
[2024-06-22 10:03:01.312] [LOG]   Sockets connected & modules started ... 
[2024-06-22 10:03:01.419] [LOG]   Launching application. 
[2024-06-22 10:03:02.467] [LOG]   Netatmo: using token.json file 
[2024-06-22 10:03:02.467] [LOG]   Netatmo: Initialized 
[2024-06-22 10:03:02.792] [LOG]   Netatmo: Authenticated 
[2024-06-22 10:03:02.793] [LOG]   Netatmo: token.json was written successfully 
[2024-06-22 10:03:02.797] [LOG]   Netatmo: New Token Expire Saturday, June 22, 2024 1:03 PM 

I will force refresh token and see what happen :=)

[2024-06-22 10:07:04.238] [LOG]   Netatmo: using token.json file 
[2024-06-22 10:07:04.238] [LOG]   Netatmo: Initialized 
[2024-06-22 10:07:04.557] [LOG]   Netatmo: Authenticated 
[2024-06-22 10:07:04.558] [LOG]   Netatmo: token.json was written successfully 
[2024-06-22 10:07:04.561] [LOG]   Netatmo: New Token Expire Saturday, June 22, 2024 1:07 PM 
[2024-06-22 10:07:19.570] [LOG]   Netatmo: Refresh Token 
[2024-06-22 10:07:19.676] [LOG]   Netatmo: token.json was written successfully 
[2024-06-22 10:07:19.676] [LOG]   Netatmo: TOKEN Updated 
[2024-06-22 10:07:19.677] [LOG]   Netatmo: New Token Expire Saturday, June 22, 2024 1:07 PM 
[2024-06-22 10:07:34.679] [LOG]   Netatmo: Refresh Token 
[2024-06-22 10:07:34.781] [LOG]   Netatmo: token.json was written successfully 
[2024-06-22 10:07:34.782] [LOG]   Netatmo: TOKEN Updated 
[2024-06-22 10:07:34.783] [LOG]   Netatmo: New Token Expire Saturday, June 22, 2024 1:07 PM 

For me, It's works :)

@bugsounet
Copy link
Contributor Author

bugsounet commented Jun 22, 2024

(Note: I really don't agree with your rules of the test workflows suite... thx to review your rules)

I suggest to review your rules:

  • Extra semicolon. : really need for a clean code
  • Strings must use singlequote. : really ? since when ?
  • Missing trailing comma. : an object must never finish with an comma
  • identifier 'xx' is not in camel case. : it's a string...
  • ...

@Lusbueb69
Copy link

Lusbueb69 commented Jun 22, 2024

@bugsounet if you want, i can share you a temporary API from my NETATMO, just send me a Message and i will send you all required codes.

at 11:15 i have cloned your fork and it works. it works after reboot the RPI, it works after pm2 restart mm. today i will test it with several RPI reboot, thank you very much to bring your skills in this project. :-)

@bugsounet
Copy link
Contributor Author

bugsounet commented Jun 22, 2024

@Lusbueb69 You can try it by your self ;)

try this in netatmo module folder

git fetch origin pull/382/head:bugsounetTest
git checkout -f bugsounetTest

If any token.json file exist ---> delete it
Connect to netatmo connect and make like readme says (like first configuration time)

next restart must use token file

To reverse to main netatmo:
in netatmo module folder

git checkout -f main

Naturally don't forget to restart MM² :)

@Lusbueb69
Copy link

i have choose an other way (because i'm a dummy-user):

  • pm2 stop mm
  • rename the existing folder netatmo to netatmo.240622
  • cd ~/MagicMirror/modules && git clone https://github.com/bugsounet/MMM-Netatmo netatmo
  • cd netatmo && npm ci --production --ignore-scripts
  • generate a new refresh-token an insert the new token in config.js
  • pm2 start mm

i'm so happy, everything works fine, still after a RPI reboot or a pm2 restart mm.

@bugsounet bugsounet marked this pull request as ready for review June 22, 2024 10:30
@bugsounet bugsounet changed the title Token read/write testing v2.1.0: Token read/write // refact code Jun 22, 2024
* delete helper.test.js / .env.template : can't works now with new
netatmo API Rules
 * add token.json in .gitignore
 * update README.md
 * minor fix in helper.js
 * cleaning package.json
 * update validation rules
 * fix renovate.json
@bugsounet
Copy link
Contributor Author

@CFenner: I'm going to stop there, it remains validate:md to do

@CFenner
Copy link
Owner

CFenner commented Jun 22, 2024

Really appreciate this PR, thank!

Your codes rules are really weird ;)

I use eslint-config-standard so that I do not need to deal with the rules configuration. One can always argue about the rules, it's just personal style that I don't want to care.

Copy link
Owner

@CFenner CFenner left a comment

Choose a reason for hiding this comment

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

👍

@CFenner CFenner merged commit 74a848b into CFenner:main Jun 22, 2024
6 of 7 checks passed
@CFenner
Copy link
Owner

CFenner commented Jun 23, 2024

@all-contributors please add @bugsounet for code, bug, review

Copy link
Contributor

@CFenner

I've put up a pull request to add @bugsounet! 🎉

@bugsounet
Copy link
Contributor Author

bugsounet commented Jun 23, 2024

@CFenner All developers have code sytle, it's sometime difficult to merge ;)

In all case, the main thing is that we managed to correct this "bug" !

I will delete my fork and inform #380 to return in your repo (if needed)

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.

3 participants