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 procon-ip to the latest repository #756

Merged
merged 175 commits into from
Aug 10, 2020
Merged

Add procon-ip to the latest repository #756

merged 175 commits into from
Aug 10, 2020

Conversation

ylabonte
Copy link
Contributor

Hi there,

as suggested by your bot (ylabonte/ioBroker.procon-ip#6 or ylabonte/ioBroker.procon-ip#7), I would like to add my adapter for the ProCon.IP pool control unit to your latest repository. Meanwhile there are also a few users that mentioned, it would be nice, to have the adapter available from your official repos. But this was inside the closed forum of the adapter's target device...

I am not clear about the process, so I think all necessary information could be taken from the adapter repository. I have tried to follow all of your checklists and guidelines for adapter development. Please just let me know if you need any further information.

Best regards
Yannic

@GermanBluefox GermanBluefox added the auto-checked This PR was automatically checked for obvious criterias label May 10, 2020
@ioBroker ioBroker deleted a comment from Apollon77 May 10, 2020
@GermanBluefox
Copy link
Contributor

Automated adapter checker

ioBroker.procon-ip

Downloads Number of Installations (latest)
NPM

👍 No errors found

  • 👀 [W400] Cannot find "procon-ip" in latest repository

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

@ioBroker ioBroker deleted a comment from ylabonte May 10, 2020
Apollon77 and others added 12 commits May 11, 2020 07:24
@ylabonte
Copy link
Contributor Author

Hi @Apollon77,
thanks for your investigation! I will transfer your feedback into one or more issues, fix all points and maybe refactor some additional parts regarding the best practices posted above. Afterwards I will resolve the merge conflict in this PR and leave a comment here again.

@ylabonte
Copy link
Contributor Author

ylabonte commented Jun 28, 2020

Just to have this mentioned: I have re-tested the current release regarding the encryption. It is correct that I totally leveraged the built-in mechanism. But I did it that way, that the desired fields are in fact encrypted using my AES encryption/decryption functions with crypto-js. Anyways I will fix this to behave as desired by ioBroker! I just wanted to explain that there is a working encryption mechanism as I promised to my users with my release notes and in a forum post. 😅️

Sample objects.json from my testing environment:

{
  "controllerUrl": "U2FsdGVkX1/gdLPf/o2lRKj9V8EPuayj4exhc9MKfkUb98HLZiWrkMViwhdpx3oS",
  "basicAuth": true,
  "username": "U2FsdGVkX1/28QEx0Cs/kAPJ9EAQpcz0E8OrL7SKUFQ=",
  "password": "U2FsdGVkX1+wkLL3K9Ke1QxU8Ps7w4e55rnkNkKigsA=",
  "updateInterval": "15000"
}

Edit: Updated the code on develop branch, to use the built-in mechanism and added compatible fallback functions (implementing the XOR technique).

@ylabonte
Copy link
Contributor Author

ylabonte commented Jun 30, 2020

Hello again,
I (hopefully?) resolved all issues arose from your feedback, created a pull request from that changes to the version you have reviewed and finally have resolved the merge conflict that meanwhile existed on this pull request.

The closed issues overview of the corresponding milestone might be a good entrypoint, if you want to review the changes in relation to the bullet points of your review.

One thing that is not visible from within the code changes: I have tested the adapter in compact mode. Simply for testing multiple instances of the adapter at once running in a single group. And since this succeeded, my productive system runs the adapter together with a harmony and a fakeroku instance in the same compact group without problems so far.

Just let me know, if there is anything else you need to know or I should do.

@GermanBluefox GermanBluefox merged commit 5cd4e47 into ioBroker:master Aug 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
must be fixed The Adapter request got review/automatic feedback that is required to be fixed before another review question Something needs to be done or answered by the adapter developer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet