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

Add mywallbox Adapter to latest #1899

Merged
merged 4 commits into from
Nov 25, 2024
Merged

Add mywallbox Adapter to latest #1899

merged 4 commits into from
Nov 25, 2024

Conversation

SKB-CGN
Copy link
Contributor

@SKB-CGN SKB-CGN commented Aug 15, 2022

Adapter for Cloud based Wallboxes like Pulsar or Pulsar Plus

@Apollon77
Copy link
Collaborator

Hi, thank you for your adapter.

Is the adapter for Pulsar Wallboxes only? Then "Wallbox" as name is too generic in my eyes ... or do you pan to add more other such? Could you please rename it to pulsar-wallbox or such?

@Apollon77 Apollon77 added the REVIEW needed (apollon77) A developer from the ioBroker Team will review the adapter, will provide comments or require changes label Aug 15, 2022
@SKB-CGN
Copy link
Contributor Author

SKB-CGN commented Aug 15, 2022

Hi, thank you for your adapter.

Is the adapter for Pulsar Wallboxes only? Then "Wallbox" as name is too generic in my eyes ... or do you pan to add more other such? Could you please rename it to pulsar-wallbox or such?

The Company is named "Wallbox" therefore i choosed this name.

@Apollon77
Copy link
Collaborator

wow ... puuhh ... a company name which matches a "in the meantime commodoty name of a device type" :-( I will discuss it in core team...
But I hope you also understand my point of view?

@SKB-CGN
Copy link
Contributor Author

SKB-CGN commented Aug 15, 2022

Sure, understand it. Never mind. They even own www.wallbox.com as their domain.
So its most common name for product and company ;)

Could you do the checks needed for the adapter in the meantime? Thanks beforehand!

@SKB-CGN
Copy link
Contributor Author

SKB-CGN commented Aug 17, 2022

Hi,
do we have some news here?

@Apollon77
Copy link
Collaborator

Apollon77 commented Aug 17, 2022

Yes, we discussed yesterday. WOuld it be an issue torename tzo "my-wallbox" so we match the name of that cloud servcie you connect to?

Additionally please state in the readme exactly that it is for Wallboxes from company "wallbox" or such ... right now also the Readme could be easiely read "works for all wallboxes". The same for io-package title/tileLang and desc fields please.

This is a special case, but we want to avoid user confusion.

Additionally please adjust the links in the repo file to use raw.githubusercontent.com like the others ... github.com is not a CDN service ...

Thnak you very much for your understanding and support on this

Changed name to My-Wallbox as requested
@SKB-CGN
Copy link
Contributor Author

SKB-CGN commented Aug 17, 2022

Hi,
would you please check the things again.

I have updated the name to My-Wallbox and changed (hopefully) all necessary dependencies and files.

Thank you!

@Apollon77
Copy link
Collaborator

I will do the adapter review in the next days ...

@SKB-CGN
Copy link
Contributor Author

SKB-CGN commented Aug 17, 2022

Great, but now i have the problem, that the adapter is not working anymore ... i do not know, whats going on.

@Apollon77
Copy link
Collaborator

Hm ... data on github looks fine ... what error you get?

Changed Name to MyWallbox
@SKB-CGN
Copy link
Contributor Author

SKB-CGN commented Aug 17, 2022

The adapter is not doing anything. No commands running, no Interval is working. Nothing.

Even not able to change to debug to see some output.

@Apollon77
Copy link
Collaborator

Code wise looks good ... also github actionsstart tghe adapter ... how you test locally?

@SKB-CGN
Copy link
Contributor Author

SKB-CGN commented Aug 17, 2022

i created the adapter with adapter creator and everytime i change the code, i do iobroker upload mywallbox

@Apollon77
Copy link
Collaborator

but you also reinstalled the adapter after that namechange completely?

@SKB-CGN
Copy link
Contributor Author

SKB-CGN commented Aug 17, 2022

Holy bible. I missed an error-handling. Will fix that shortly.

@SKB-CGN
Copy link
Contributor Author

SKB-CGN commented Aug 17, 2022

Now its working again. Thanks for waiting.

@SKB-CGN
Copy link
Contributor Author

SKB-CGN commented Aug 31, 2022

@Apollon77 Do we have some news here? :)

@Apollon77
Copy link
Collaborator

Yes, after vacation, wasn't able to do it before

@GermanBluefox GermanBluefox added auto-checked This PR was automatically checked for obvious criterias must be fixed The Adapter request got review/automatic feedback that is required to be fixed before another review labels Sep 6, 2022
@ioBroker ioBroker deleted a comment from Apollon77 Sep 6, 2022
@SKB-CGN
Copy link
Contributor Author

SKB-CGN commented Sep 16, 2022

Can someone please tell me, how "ioBroker.my-wallbox" can be removed here? I needed to change the name and the check always goes through my-wallbox either instead of only using mywallbox.

Thank you!

@Apollon77
Copy link
Collaborator

All good ... the checker only checks the initial repo file change ... so clean would be new PR ... but in this case "known", so all fine

@SKB-CGN
Copy link
Contributor Author

SKB-CGN commented Sep 16, 2022

Ok and all things are good now?

@Apollon77
Copy link
Collaborator

Hi,
here my review comments:

Thank you for checking and adjusting,

Ingo

@mcm1957 mcm1957 added the RE-REVIEW pending (by mcm1957) Changes requested by review have been applied, re-review could be done. label Sep 18, 2024
@mcm1957
Copy link
Collaborator

mcm1957 commented Nov 1, 2024

RE-CHECK!

@github-actions github-actions bot added the must be fixed The Adapter request got review/automatic feedback that is required to be fixed before another review label Nov 1, 2024
@github-actions github-actions bot deleted a comment from SKB-CGN Nov 1, 2024
Copy link

github-actions bot commented Nov 1, 2024

Automated adapter checker

blob

Downloads - Test and Release
NPM

  • ❗ [E000] FATAL: cannot access repository https://api.github.com/repos/SKB-CGN/ioBroker.wallbox/blob
  • ❗ [E999] GLOBAL ERROR: AxiosError: Request failed with status code 404, {"message":"Request failed with status code 404","name":"AxiosError","stack":"AxiosError: Request failed with status code 404\n at settle (/home/runner/work/ioBroker.repositories/ioBroker.repositories/node_modules/axios/dist/node/axios.cjs:2019:12)\n at Unzip.handleStreamEnd (/home/runner/work/ioBroker.repositories/ioBroker.repositories/node_modules/axios/dist/node/axios.cjs:3135:11)\n at Unzip.emit (node:events:529:35)\n at endReadableNT (node:internal/streams/readable:1400:12)\n at process.processTicksAndRejections (node:internal/process/task_queues:82:21)\n at Axios.request (/home/runner/work/ioBroker.repositories/ioBroker.repositories/node_modules/axios/dist/node/axios.cjs:4287:41)\n at process.processTicksAndRejections (node:internal/process/task_queues:95:5)","config":{"transitional":{"silentJSONParsing":true,"forcedJSONParsing":true,"clarifyTimeoutError":false},"adapter":["xhr","http","fetch"],"transformRequest":[null],"transformResponse":[null],"timeout":0,"xsrfCookieName":"XSRF-TOKEN","xsrfHeaderName":"X-XSRF-TOKEN","maxContentLength":-1,"maxBodyLength":-1,"env":{},"headers":{"Cache-Control":"no-cache","Pragma":"no-cache","Expires":"0","User-Agent":"axios/1.7.7","Accept-Encoding":"gzip, compress, deflate, br"},"cache":false,"method":"get","url":"https://api.github.com/repos/SKB-CGN/ioBroker.wallbox/blob"},"code":"ERR_BAD_REQUEST","status":404}

ioBroker.my-wallbox

Downloads - Test and Release
NPM

  • ❗ [E020] Name of adapter in package.json must be lowercase and be equal to "iobroker.my-wallbox". Now is "iobroker.mywallbox"
  • ❗ [E103] "common.name" in io-package.json must be equal to "my-wallbox'". Now is mywallbox
  • ❗ [E251] Bluefox was not found in the collaborators on NPM!. Please execute in adapter directory: npm owner add bluefox iobroker.my-wallbox
  • ❗ [E999] GLOBAL ERROR: TypeError: Cannot read properties of undefined (reading 'latest'), {}
  • 👀 [W034] @iobroker/adapter-core 3.1.6 specified. 3.2.2 is recommended. Please consider updating dependencies at package.json

ioBroker.mywallbox

Downloads Number of Installations (latest) - Test and Release
NPM

  • ❗ [E254] Versions "0.0.17, 0.0.16" listed at common.news at io-package.json do not exist at NPM. Please remove from news section.
  • 👀 [S522] Please consider migrating to admin 5 UI (jsonConfig).
  • 👀 [S526] Consider adding plugin "@alcalzone/release-script-plugin-manual-review".
  • 👀 [W034] @iobroker/adapter-core 3.1.6 specified. 3.2.2 is recommended. Please consider updating dependencies at package.json
  • 👀 [W401] Cannot find "mywallbox" in latest repository

Add comment "RE-CHECK!" to start check anew

@SKB-CGN
Copy link
Contributor Author

SKB-CGN commented Nov 11, 2024

RE-CHECK!

@github-actions github-actions bot added the *📬 a new comment has been added label Nov 11, 2024
Copy link

Automated adapter checker

blob

Downloads - Test and Release
NPM

  • ❗ [E000] FATAL: cannot access repository https://api.github.com/repos/SKB-CGN/ioBroker.wallbox/blob
  • ❗ [E999] GLOBAL ERROR: AxiosError: Request failed with status code 404, {"message":"Request failed with status code 404","name":"AxiosError","stack":"AxiosError: Request failed with status code 404\n at settle (/home/runner/work/ioBroker.repositories/ioBroker.repositories/node_modules/axios/dist/node/axios.cjs:2019:12)\n at Unzip.handleStreamEnd (/home/runner/work/ioBroker.repositories/ioBroker.repositories/node_modules/axios/dist/node/axios.cjs:3135:11)\n at Unzip.emit (node:events:529:35)\n at endReadableNT (node:internal/streams/readable:1400:12)\n at process.processTicksAndRejections (node:internal/process/task_queues:82:21)\n at Axios.request (/home/runner/work/ioBroker.repositories/ioBroker.repositories/node_modules/axios/dist/node/axios.cjs:4287:41)\n at process.processTicksAndRejections (node:internal/process/task_queues:95:5)","config":{"transitional":{"silentJSONParsing":true,"forcedJSONParsing":true,"clarifyTimeoutError":false},"adapter":["xhr","http","fetch"],"transformRequest":[null],"transformResponse":[null],"timeout":0,"xsrfCookieName":"XSRF-TOKEN","xsrfHeaderName":"X-XSRF-TOKEN","maxContentLength":-1,"maxBodyLength":-1,"env":{},"headers":{"Cache-Control":"no-cache","Pragma":"no-cache","Expires":"0","User-Agent":"axios/1.7.7","Accept-Encoding":"gzip, compress, deflate, br"},"cache":false,"method":"get","url":"https://api.github.com/repos/SKB-CGN/ioBroker.wallbox/blob"},"code":"ERR_BAD_REQUEST","status":404}

ioBroker.my-wallbox

Downloads - Test and Release
NPM

  • ❗ [E020] Name of adapter in package.json must be lowercase and be equal to "iobroker.my-wallbox". Now is "iobroker.mywallbox"
  • ❗ [E103] "common.name" in io-package.json must be equal to "my-wallbox'". Now is mywallbox
  • ❗ [E251] Bluefox was not found in the collaborators on NPM!. Please execute in adapter directory: npm owner add bluefox iobroker.my-wallbox
  • ❗ [E999] GLOBAL ERROR: TypeError: Cannot read properties of undefined (reading 'latest'), {}

ioBroker.mywallbox

Downloads Number of Installations (latest) - Test and Release
NPM

👍 No errors found

  • 👀 [S522] Please consider migrating to admin 5 UI (jsonConfig).
  • 👀 [S526] Consider adding plugin "@alcalzone/release-script-plugin-manual-review".
  • 👀 [W401] Cannot find "mywallbox" in latest repository

Add comment "RE-CHECK!" to start check anew

@github-actions github-actions bot deleted a comment from SKB-CGN Nov 11, 2024
@SKB-CGN
Copy link
Contributor Author

SKB-CGN commented Nov 11, 2024

@mcm1957 As all errors have been fixed, can we proceed?

@mcm1957
Copy link
Collaborator

mcm1957 commented Nov 11, 2024

Yes i wll do as soon as I find time.
Have some backlog due to travel to Solingen (Meeting) and back.
Sorry.

@SKB-CGN
Copy link
Contributor Author

SKB-CGN commented Nov 11, 2024

Not a problem ;)
Take your time!

@mcm1957 mcm1957 added new at LATEST and removed new at LATEST *📬 a new comment has been added labels Nov 24, 2024
Copy link

github-actions bot commented Nov 24, 2024

ioBroker repository information about New at LATEST tagging

  • Please ignore this comment at 'old' PRs.

@mcm1957
Copy link
Collaborator

mcm1957 commented Nov 24, 2024

@SKB-CGN

First of all - THANK YOU for the time and effort you spend to maintain this adapter.

I'm sorry, that I did not check this adapter for such a long time - I simpyl do not have enough spare time.

Thanks for fixing issues already listed by Apollon77 and adapter checker. I do not remeber to have scanned this adapter in the past so I did a full recheck. Sorry to say that I would like to note some MINOR remarks. I think they should be fixable with small effort. Please take a look and do not spent time for eslint - this last point is only a hint for the future and not related to release for latest.

Thanks
mcm1957


  • folder ./lib contains unrelated files

    The folder ./lib contains several fiels copied from ioBroker.repositories. They do not make sense there. Please delete them (I think all but states.js are incorrect)

  • creation of state tree seems to be incomplete

    You are creating some states with a dot in their name, i.e. 'lock.auto_lock'. This creates a tree consisting of an folder named lock and a state below named auto_lock. I could not detect the place where the (folder) Object 'lock' is created. Please note that all levels ob objects (device, channel, folders, states) must be explicitly created. Missing to create intermediate levels might be considered an error in furure versions of js-controller.

    As you seem to have fixed states only, it would be fully sufficient to liste the folders / channels (and even the states) at io-package. See existing creation of info and info.connection state.

  • small note to state roles

    Only the values specified here (https://github.com/ioBroker/ioBroker.docs/blob/master/docs/en/dev/stateroles.md) may be used as state roles. Do not invent new names as this might disturb the functionalyity of other adapters or vis.

    In addition the roles MUST match the read/write functionality. As an example a role value.* requires that the state is a readable state. Input states (write only) should have roles like level.*. Please read the description carefully. States with role 'button' should (must) have attribute 'common.read=false' set. A button ( "Taster" in german only triggers when you press but else has no state), so it should also be not readable. This is also like HomeMatic does it. A switch has clear states true or false and so can be read.

    Please avoid using general roles (state, value) whenever a dedicated role exists.

    State 'locked' must not have role 'indicator' if it is writeable. So either (suggestion) use role 'switch' or disable writeing the state - whatever matches the functionality. (https://github.com/SKB-CGN/ioBroker.mywallbox/blob/3c8d70edebf1902f658aa918aea6a8318badc882/lib/states.js#L413)

    Input states (i.e. maxChargingCurrent) must not have role value.* (as this is read only) but level.* - https://github.com/SKB-CGN/ioBroker.mywallbox/blob/3c8d70edebf1902f658aa918aea6a8318badc882/lib/states.js#L435

  • A hint for migration of EsLint (not blocking remark)

    If you plan to migrate EsLint from 8.x.x to current release 9.x.x please migrate to use @iobroker/eslint-config. This is a new central configurarion of eslint-rules which should be used for all (new) adapters. But its not mandatory. A migration / installation guide is can be found here:

https://github.com/ioBroker/ioBroker.eslint-config/blob/main/MIGRATION.md

Thanks for reading and evaluating this suggestions.
McM1957

Please add a comment when you have reviewed and fixed the suggestionsor at least commented the suggestions and you think the adapter is ready for a re-review!

reminder 1.12.2024

@mcm1957 mcm1957 removed the RE-REVIEW pending (by mcm1957) Changes requested by review have been applied, re-review could be done. label Nov 24, 2024
@github-actions github-actions bot added 1.12.2024 *📬 a new comment has been added and removed 30.9.2024 labels Nov 24, 2024
@SKB-CGN
Copy link
Contributor Author

SKB-CGN commented Nov 25, 2024

Hi,
may i ask you, to check this again, please?

I didnt find out, to state "normal" seconds state - is it possible, to use "value.interval" for this?
It displays a total charging time - which is in seconds.

@mcm1957 mcm1957 added RE-REVIEW pending (by mcm1957) Changes requested by review have been applied, re-review could be done. lgtm Looks Good To Me and removed *📬 a new comment has been added RE-REVIEW pending (by mcm1957) Changes requested by review have been applied, re-review could be done. labels Nov 25, 2024
@mcm1957
Copy link
Collaborator

mcm1957 commented Nov 25, 2024

I do not see a normal "seconds" state for timing too. You may use value.intervall - although its no true interval - or simüly use value as role.

In general it looks like all issues are satisifed. I'll rekease the adapter to stable now.

THANKS a lot for your work.

@mcm1957 mcm1957 merged commit 6067d96 into ioBroker:master Nov 25, 2024
99 checks passed
@mcm1957
Copy link
Collaborator

mcm1957 commented Nov 25, 2024

This adapter has been released to latest repository and should be visible within 24h maximum.

Please create a thread at https://forum.iobroker.net/category/91/tester titled like "Test Adapter " to collect some user feedback and provide a link to this topic when requesting addition to stable repository later.

Note: If an other testing topic already exists, it' OK to continue using this topic too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-checked This PR was automatically checked for obvious criterias lgtm Looks Good To Me must be fixed The Adapter request got review/automatic feedback that is required to be fixed before another review new at LATEST
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants