-
-
Notifications
You must be signed in to change notification settings - Fork 24
fix(ac): refactor to add new binary power format #411
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
Conversation
|
thanks for your PR. we may need to pass the github action. |
|
Well, TL;DR;, I fixed the typing and formatting issues, and also some old bugs, like a bad length check (same variable was assigned twice), and removed some redundant/unnecessary code (as highlighted by ruff / mypy / VSCode / etc and confirmed to be OK). @wuwentao I've spent like at least an hour trying to setup VSCode on my Win 11, with all the plugins and everything, but it was a pain to get that all running with the devcontainer running here. Checking your Dockerfile, it seems to be related to your pip mirror configuration, because of which it attempts connecting to -pypi.org (leading dash): Dockerfile - looks OK: devcontainer.json - Idk how parameter expansion works when VSCode loads devcontainer.json, like I know bash would substitute So a Windows CRLF is causing trouble - Found the shebang in script/setup.sh and changed that file from CRLF to LF. Funny enough, all my files are marked as modified now suddenly, and ofc it's the Windows CRLF line-endings everywhere. Don't know what changed the core.autocrlf setting, would have needed that before already, but it's something to be mentioned for other devs (making sure they check this out with LF). Maybe you could consider writing up a brief CONTRIBUTING.md, with what all you expect to be setup/run and how, before creating a PR? |
|
@folfy sorry, as most of us under GFW in china network, we can't directly access github/pip/docker/etc, so we need to config a local mirror about these sites: for windows user, WSL2 should be the expected solution with VScode + devcontainer. install a ubuntu under windows WS2, all these feature should works. |
|
There is some testcases for 0xAC device. |
VSCode/devcontainer setup and proxy config
Yeah, I already had WSL2 with Ubuntu installed, but still ran into all these issues mentioned above. At least the devcontainer.json should be fixed: Documentation
Thanks, yeah, I'll check later on today or tomorrow 👍 Tests
Pylint config issueUnfortunately tests don't seem to run here - I think cause I got some other Pylint error here still, that I didn't bother to investigate before: Seems your setting pylintrc:25 ruff issueAlso this (typical Windows) issue raised by ruff is probably still blocking my tests: Guess it's copying the checkout from Windows/NTFS to WSL, where NTFS doesn't have the executable flag, and correspondingly flags every file executable when copying or mount. Dunno how to fix that yet. Ignoring it though VSCode doesn't seem to work... @wuwentao Are you running VSCode natively on Windows? I usually develop on my headless server with vim and co., so not bothering with a graphical environment there. Thought if I need to use VSCode I best run it on Windows, but seems to cause a lot of issues. Failed testYes, this test also is flawed, just like the function itself was:
Since I fixed that, I'll had to correct the two tests in message_ac_test.py line 668 and 677 (shift the highest byte only by 3 bytes / 24 bits instead of 32). Also created a new test-case for method 12, re-using the test for method 2. Hope it's good now, due to the ruff issue above I still can't run tests... 😞 |
|
Third time's the charm, now it should be all good - Proposed fixes for devcontainer / pylint in #415 |
…on (#415) Proposal, as highlighted in #411 (comment) ### VSCode/devcontainer setup and proxy config > @folfy sorry, as most of us under GFW in china network, we can't directly access github/pip/docker/etc, so we need to config a local mirror about these sites: ` export APT_MIRROR_DOMAIN=xxxxx`, or redirect these domain to proxy. > > for windows user, **WSL2** should be the expected solution with VScode + devcontainer. install a ubuntu under windows WS2, all these feature should works. finally, I will try to add some info in CONTRIBUTING.md. Yeah, I already had WSL2 with Ubuntu installed, but still ran into all these issues mentioned above. At least the devcontainer.json should be fixed: `"PIP_MIRROR_DOMAIN": "${localEnv:PIP_MIRROR_DOMAIN:-pypi.org}"` I found out that "localEnv:PIP_MIRROR_DOMAIN" is a docker (compose?) directive to get this from the users environment. Not sure where / how the default value is expanded, but removing the dash after the colon fixes the issue (using the last value as default), so proposing that fix. ### Pylint config issue Unfortunately tests don't seem to run here - I think cause I got some other Pylint error here still, that I didn't bother to investigate before: ``` Unrecognized option found: suggestion-mode Pylint(E0015:unrecognized-option) ``` Seems your setting pylintrc:25 `suggestion-mode=yes` has been deprecated in Pylint 4.0: https://pylint.readthedocs.io/en/latest/whatsnew/4/4.0/index.html So I removed that option.
|
@folfy sorry, I'm always using Mac as my working PC or ssh to a Ubuntu Linux remote server, and just did some basic testing with windows PC and WSL2 last year. In addition, my daily work is too busy these months, so I don't have enough time to process new feature issue/add new feature in this project, anyway, I will continue to do some when time is enough. |
merged AC energy / power functions
-> Rewrote these and corresponding calls and helper-functions, to make them more versatile / deduplicate common code
Fixed issue wuwentao/midea_ac_lan#678: AC energy values are x10 of real value (Midea PortaSplit)
-> Introduced format BINARY1 (power_analysis_method 12 = variant of method 2)
Lower digit still represents existing base-formats (BCD, binary, mixed/int), while higher digit(s) can be used to represent different precisions/scalars
Tested changes locally: power_analysis_method 2 and 12 (also got a Midea PortaSplit, actually needing mode 12)
Also, this fixes a bug with energy values >= 655.35 kWh (mode 12) / 6553.5 kWh (existing mode 2), line 1117:
return float((byte1 << 32) + (byte2 << 16) + (byte3 << 8) + byte4) / 10-> byte1 should be shifted 24 instead of 32 bits
Change needs to be considered in documentation in https://github.com/wuwentao/midea_ac_lan/blob/master/doc/AC.md
Also document it's meaning of interpretation, since it's helpful when migrating from Midea Smart AC (mill1000/midea-ac-py):