-
Notifications
You must be signed in to change notification settings - Fork 290
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 adapter flexcharts to latest #3889
Conversation
Thanks for spending your time and providing a new adapter for ioBroker. Your adapter will get a manual review as soon as possible. Please stand by - this might last one or two weeks. Feel free to continue your work and create new releases. You do NOT need to close or update this PR in case of new releases. In the meantime please check any feedback issues logged by automatic adapter checker and try to fix them. You will find the results of the review and eventually issues / suggestings as a comment to this PR. So please keep this PR watched. If you have any urgent questions feel free to ask. reminder 13.8.2024 |
First of all - THANK YOU for the time and effort you spend to maintain this adapter. I would like to give some feedback based on my personal oppinion. @Apollon77 might have additional suggestions or even a different oppinion to one or the other statement. Please feel free to contact him if you cannot follow my suggestions or want to discuss some special aspects. Please note that most likely @foxriver76 or @bluefox might give some remarks too.
To be discussed with @foxriver76 and @Apollon77
Thanks for reading and evaluating this suggestions. 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 15.9.2024 |
Thanks a lot for doing the review! Hopefully, I fixed the findings so far. Regarding ECharts licence: It appies "Apache License, Version 2.0". To my understanding usage of local copy is allowed. I added a note in License part of Readme. Regarding basic concept: Yes, there is no refresh mechanism on itself. It's quite possible that the current approach isn't optimal for this concept. Unfortunately, I don't know anything about programming widgets etc., so I can't assess these alternatives. |
@mcm1957 Hi Martin, I would appreciate it if we could come to a decision on how to proceed here. If it's not feasable to use an adapter I would need your advice for a better approach.
Finally an EChart is shown according to users definition. It's possible to use almost full feature set of Apache ECharts within ioBrokers eco system. |
Hi, and sorry for the delay in answering. In general we would have some proposals how to make the adapter more generically working, also e.g. with the ioBroker cloud, because this is not possible with the current "own port" approach. So one proposal would be to rebuild it as a web extension, so that the web adapter server can be used and your adapter kind of have a sub path in that for it's things and do not need an own port. Via this web server you also have the default websocket interface for web to fetch states or send messages, so also this might already be reusable. The conversion to a web extension should be not too hard and the logic can be check e.g. in the simpleapi or socketio or other adapters. Also some informations are in the web adapter readme. But in fact this is mainly a proposal to be better usable for the users. Rest is fine and in my eyes we do not want to block the repo addition any longer. So your decision if you like to do web extension first and we wait, or if you want to do it later (or never ;-)) and we add now... Ingo |
reminder 8.10.2024 |
Please ignore E166 - seems to be false positive |
@MyHomeMyData Please ignore checker errors until the recommended combination is clearified |
It's me again: I checked oth aapters configures as mode:extension: ioBroker.flexcharts.json The correct setup seems to be:
I suggest to setup flexcharts this wai for now - if you have time. I cannot ensire that his setup will be correct but as the listed adpaters have such a setup (check yourself if interested) it should work. Foxriver will try to analyze the controlelr code and respond within the next week - at least if nothing important arises in between. |
@mcm1957 I changed configuration according to your proposal. Hope this fits for the moment. Now instance is displayed as "running" and gray color. Fine from my point of view. |
Please ignore errors for now - these are false positives |
Automated adapter checkerioBroker.flexcharts👍 No errors found
Add comment "RE-CHECK!" to start check anew |
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. |
No description provided.