Ewelink discover fix and enhancements #1044

NickDub-old · 2021-01-16T14:55:40Z

Pull Request check-list

To ensure your Pull Request can be accepted as fast as possible, make sure to review and check all of these items:

If your changes affects code, did your write the tests?
Are tests passing? (npm test on both front/server)
Is the linter passing? (npm run eslint on both front/server)
Did you run prettier? (npm run prettier on both front/server)
If you are adding a new features/services, did you run integration comparator? (npm run compare-translations on front)
If your changes modify the API (REST or Node.js), did you modify the API documentation? (Documentation is based on comments in code)
If you are adding a new features/services which needs explanation, did you modify the user documentation? See the GitHub repo and the website.
Did you add fake requests data for the demo mode (front/src/config/demo.json) so that the demo website is working without a backend? (if needed) See https://demo.gladysassistant.com.

NOTE: these things are not required to open a PR and can be done afterwards / while the PR is open.

Description of change

Please provide a description of the change here. It's always best with screenshots, so don't hesitate to add some!

codecov · 2021-01-16T15:09:43Z

Codecov Report

Merging #1044 (48cb671) into master (213ba3b) will decrease coverage by 0.02%.
The diff coverage is 97.60%.

@@            Coverage Diff             @@
##           master    #1044      +/-   ##
==========================================
- Coverage   96.66%   96.64%   -0.03%     
==========================================
  Files         686      684       -2     
  Lines        8970     8990      +20     
==========================================
+ Hits         8671     8688      +17     
- Misses        299      302       +3

Impacted Files	Coverage Δ
server/services/ewelink/lib/features/humidity.js	`90.90% <90.90%> (ø)`
...erver/services/ewelink/lib/features/temperature.js	`90.90% <90.90%> (ø)`
server/services/ewelink/lib/device/poll.js	`97.82% <96.96%> (-2.18%)`	⬇️
server/lib/device/device.pollAll.js	`100.00% <100.00%> (ø)`
server/services/ewelink/lib/device/discover.js	`100.00% <100.00%> (ø)`
server/services/ewelink/lib/device/setValue.js	`100.00% <100.00%> (ø)`
server/services/ewelink/lib/features/binary.js	`100.00% <100.00%> (ø)`
server/services/ewelink/lib/features/index.js	`100.00% <100.00%> (ø)`
server/services/ewelink/lib/utils/constants.js	`100.00% <100.00%> (ø)`
server/services/ewelink/lib/utils/externalId.js	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 213ba3b...48cb671. Read the comment docs.

atrovato

Looks fine to me (code only), didn't try in real mode.

I really like the fact you upgrade the eWelink service package version :)

One question, my device is offline, why can't I add it to Gladys?

server/services/ewelink/lib/device/poll.js

server/services/ewelink/lib/features/energyPower.js

server/services/ewelink/lib/features/index.js

NickDub-old · 2021-02-22T18:45:15Z

One question, my device is offline, why can't I add it to Gladys?

If the device is offline, we cannot detect the available features and therefore, for the moment, we cannot add it.
In the future, I will add the ability to add and update an unhandled device.

atrovato

Looks good to me, but I don't have any ewelink device anymore, maybe @VonOx have some to test it in real life.

Pierre-Gilles · 2021-03-01T05:26:00Z

@VonOx could you try this ? :) Looks good to me too, looking for a real world review before merging

atrovato · 2021-03-10T14:43:51Z

@VonOx do you still have eWelink devices?

VonOx · 2021-03-10T21:16:00Z

Yes sorry guys for delay, will test this week end.

VonOx · 2021-03-12T16:30:01Z

Some testing ( With sonoff basic ):

Account setup OK
Device discover ok
Device added to dashboard ok ( but initial state was wrong , displayed on but was off )
Switch to off , nothing happen ( ok because initial state was wrong )
Turn on ( OK )
Turn off ( NOK ) - Displayed off in dashboard but device is on
After waiting device poll, real state is displayed in dashboard
Turn off OK

Maybe polling related problem or maybe I'm too fast while testing
PR Seem's ok

EDIT: Got this error from nowhere after testing ( ❤️ ITEAD server )

2021-03-12T17:34:37+0100 <error> device.poll.js:23 (DeviceManager.poll) There was an error while polling device ewelink-1000fb237d
2021-03-12T17:34:37+0100 <error> device.poll.js:24 (DeviceManager.poll) Error403: eWeLink: Authentication failed
    at EweLinkHandler.throwErrorIfNeeded (/home/vonox/repos/GladysEwilink/server/services/ewelink/lib/device/index.js:57:13)
    at EweLinkHandler.poll (/home/vonox/repos/GladysEwilink/server/services/ewelink/lib/device/poll.js:68:14)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)
    at async DeviceManager.poll (/home/vonox/repos/GladysEwilink/server/lib/device/device.poll.js:21:5) {
  status: 403,
  code: 'FORBIDDEN'
}

Pierre-Gilles · 2021-03-15T02:30:10Z

Thanks for your testing @VonOx :)

Device added to dashboard ok ( but initial state was wrong , displayed on but was off )
Switch to off , nothing happen ( ok because initial state was wrong )

So it seems there is an initial state problem? So PR is ok or not ? ^^

NickDub-old · 2021-04-01T16:48:13Z

The pr is OK, but there are vulnerabilities.
Can you check it ?

atrovato

Ok looks good to me.

Pierre-Gilles

Hi! Thanks for your PR :)

I wrote a few very minor changes related to code only.

On the functional side, was this PR tested properly ?

The bug reported by @VonOx in previous messages were fixed ?

If it's all good, and it has been tested, I'm good to merge! 🙂

server/services/ewelink/lib/device/index.js

server/services/ewelink/lib/device/discover.js

Pierre-Gilles · 2021-06-07T05:55:33Z

Hello! Just saw your changes, looks better!

On the functional side, was this PR tested properly ?

The bug reported by @VonOx in previous messages were fixed ?

NickDub-old · 2021-06-07T16:20:12Z

The bug reported by @VonOx in previous messages were fixed ?

Not yet. I'm trying to reproduce it...
Otherwise, everything is OK.

Pierre-Gilles · 2021-10-11T03:16:31Z

Hello! Just wanted to know the status of this PR ? :)

If you need help, don't hesitate to ping me on the forum.

NickDub-old · 2021-10-13T17:50:36Z

I don't have any more ewelink devices to test, but if it's ok for you, this PR is ready to be merged...

VonOx · 2021-10-13T21:51:37Z

I have one, can test if you need.

Pierre-Gilles · 2021-10-14T05:06:04Z

OK @VonOx Let me know if the bug you previously saw is indeed fixed :)

VonOx · 2021-10-19T16:00:32Z

@Pierre-Gilles Not reproduced , so yes it's fixed

Pierre-Gilles · 2021-10-22T03:29:28Z

Do you confirm that this PR is retro-compatible with existing connected ewelink device ?

There is no breaking change, like users don't need to re-connect their device to make them work ?

External_id used did not change?

NickDub-old · 2021-10-22T18:06:54Z

There is no change to the External_id.
This PR is retro-compatible with existing connected ewelink device ;)

Pierre-Gilles

Thanks for the PR then! :)

I'm merging this !

relativeci · 2021-10-25T09:35:08Z

Job #132: Bundle Size — 6.67MB (~-0.01%).

a755d55 vs 213ba3b

Changed metrics (2/8)

Metric	Current	Baseline
Initial JS	`2.87MB`(`~-0.01%`)	`2.87MB`
Cache Invalidation	`42.81%`	`0%`

Changed assets by type (1/7)

	Current	Baseline
JS	`4.76MB` (`~-0.01%`)	`4.76MB`

View Job #132 report on app.relative-ci.com

NickDub-old force-pushed the ewelink-discover-fix branch 3 times, most recently from ca44317 to 5c375d0 Compare January 17, 2021 21:23

NickDub-old force-pushed the ewelink-discover-fix branch 3 times, most recently from 8687431 to 953dc52 Compare February 2, 2021 18:44

NickDub-old force-pushed the ewelink-discover-fix branch from 953dc52 to bc828b8 Compare February 9, 2021 18:45

NickDub-old force-pushed the ewelink-discover-fix branch from bc828b8 to 2fbf431 Compare February 18, 2021 18:52

NickDub-old marked this pull request as ready for review February 18, 2021 18:53

NickDub-old requested a review from Pierre-Gilles February 18, 2021 20:10

atrovato reviewed Feb 20, 2021

View reviewed changes

atrovato previously approved these changes Feb 22, 2021

View reviewed changes

NickDub-old dismissed atrovato’s stale review via 9aee8fd March 16, 2021 17:35

NickDub-old force-pushed the ewelink-discover-fix branch from e8f52b4 to 9aee8fd Compare March 16, 2021 17:35

NickDub-old force-pushed the ewelink-discover-fix branch 3 times, most recently from ab7f15e to 16bee9f Compare March 31, 2021 11:56

NickDub-old force-pushed the ewelink-discover-fix branch from 16bee9f to 7085bf5 Compare April 7, 2021 16:55

NickDub-old force-pushed the ewelink-discover-fix branch from 7085bf5 to 9f765bd Compare May 19, 2021 18:17

atrovato previously approved these changes May 23, 2021

View reviewed changes

Pierre-Gilles requested changes May 27, 2021

View reviewed changes

server/services/ewelink/lib/device/index.js Outdated Show resolved Hide resolved

server/services/ewelink/lib/device/index.js Outdated Show resolved Hide resolved

server/services/ewelink/lib/device/discover.js Outdated Show resolved Hide resolved

NickDub-old dismissed atrovato’s stale review via 6a424fe June 1, 2021 13:48

NickDub-old force-pushed the ewelink-discover-fix branch 2 times, most recently from 6a424fe to e78df21 Compare June 4, 2021 12:38

NickDub added 4 commits October 13, 2021 19:26

Rewrite service to fix bug and add enhancements

877dd65

Add enhancements to the front

89cd1d5

Change in feature management

dfdda98

Add change from review

725b6cb

NickDub-old force-pushed the ewelink-discover-fix branch from e78df21 to 725b6cb Compare October 13, 2021 17:26

Merge branch 'master' into ewelink-discover-fix

48cb671

Pierre-Gilles approved these changes Oct 25, 2021

View reviewed changes

Pierre-Gilles merged commit a755d55 into GladysAssistant:master Oct 25, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ewelink discover fix and enhancements #1044

Ewelink discover fix and enhancements #1044

NickDub-old commented Jan 16, 2021 •

edited

codecov bot commented Jan 16, 2021 •

edited

atrovato left a comment

NickDub-old commented Feb 22, 2021

atrovato left a comment

Pierre-Gilles commented Mar 1, 2021

atrovato commented Mar 10, 2021

VonOx commented Mar 10, 2021

VonOx commented Mar 12, 2021 •

edited

Pierre-Gilles commented Mar 15, 2021 •

edited

NickDub-old commented Apr 1, 2021 •

edited

atrovato left a comment

Pierre-Gilles left a comment

Pierre-Gilles commented Jun 7, 2021

NickDub-old commented Jun 7, 2021

Pierre-Gilles commented Oct 11, 2021

NickDub-old commented Oct 13, 2021

VonOx commented Oct 13, 2021

Pierre-Gilles commented Oct 14, 2021

VonOx commented Oct 19, 2021

Pierre-Gilles commented Oct 22, 2021

NickDub-old commented Oct 22, 2021

Pierre-Gilles left a comment

relativeci bot commented Oct 25, 2021

Navigation Menu

Ewelink discover fix and enhancements #1044

Ewelink discover fix and enhancements #1044

Conversation

NickDub-old commented Jan 16, 2021 • edited

Pull Request check-list

Description of change

codecov bot commented Jan 16, 2021 • edited

Codecov Report

atrovato left a comment

Choose a reason for hiding this comment

NickDub-old commented Feb 22, 2021

atrovato left a comment

Choose a reason for hiding this comment

Pierre-Gilles commented Mar 1, 2021

atrovato commented Mar 10, 2021

VonOx commented Mar 10, 2021

VonOx commented Mar 12, 2021 • edited

Pierre-Gilles commented Mar 15, 2021 • edited

NickDub-old commented Apr 1, 2021 • edited

atrovato left a comment

Choose a reason for hiding this comment

Pierre-Gilles left a comment

Choose a reason for hiding this comment

Pierre-Gilles commented Jun 7, 2021

NickDub-old commented Jun 7, 2021

Pierre-Gilles commented Oct 11, 2021

NickDub-old commented Oct 13, 2021

VonOx commented Oct 13, 2021

Pierre-Gilles commented Oct 14, 2021

VonOx commented Oct 19, 2021

Pierre-Gilles commented Oct 22, 2021

NickDub-old commented Oct 22, 2021

Pierre-Gilles left a comment

Choose a reason for hiding this comment

relativeci bot commented Oct 25, 2021

Job #132: Bundle Size — 6.67MB (~-0.01%).

NickDub-old commented Jan 16, 2021 •

edited

codecov bot commented Jan 16, 2021 •

edited

VonOx commented Mar 12, 2021 •

edited

Pierre-Gilles commented Mar 15, 2021 •

edited

NickDub-old commented Apr 1, 2021 •

edited