Skip to content
This repository has been archived by the owner on Sep 8, 2024. It is now read-only.

Unshadow Message class in _handle_pairing_complete #2790

Merged

Conversation

forslund
Copy link
Collaborator

@forslund forslund commented Jan 4, 2021

Description

Fixes issue spotted by @j1nx, the "Message" class is shadowed by the unused "Message" argument.

Pairing (previously at least) muted the mic until pairing was complete. The restoration of the mic state has been broken (it seems) since the Mark-2 work began. Which leads me to believe that the initial mute of the mic isn't working.

This will not make things work worse and the code will do what it tries to do but if the mute during pairing is something we don't want in the future we should just remove this handler.

Contributor license agreement signed?

CLA [ Yes ]

Fixes a small typo making the unmute after first pairing call fail.
@devops-mycroft devops-mycroft added the CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors) label Jan 4, 2021
@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

Copy link
Contributor

@krisgesling krisgesling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good fix, nice and simple.

@krisgesling
Copy link
Contributor

This is a good fix for the immediate issue so merging it in.

It does make me question if there is a reason this unmute isn't called from the Pairing Skill itself?

@krisgesling krisgesling merged commit c0dac56 into MycroftAI:dev Jan 5, 2021
@forslund
Copy link
Collaborator Author

forslund commented Jan 5, 2021

I think the original reason that it was put in the enclosure was that it would depend on the platform whether it was needed to mute / unmute the mic during initial setup.

@j1nx
Copy link

j1nx commented Jan 5, 2021

Indeed, mute at that part wasn't working. The Hey Mycroft demo sentences at the end triggered the device.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants