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

[input] Re-factor remote control #13502

Merged
merged 4 commits into from
Feb 9, 2018
Merged

Conversation

afedchin
Copy link
Member

@afedchin afedchin commented Feb 8, 2018

in order of preparation to an implementation of support remote control on xbox I've made refactoring current code.

@afedchin afedchin added Type: Cleanup non-breaking change which removes non-working or unmaintained functionality Component: Input v18 Leia labels Feb 8, 2018
@afedchin afedchin added this to the L 18.0-alpha1 milestone Feb 8, 2018
@afedchin afedchin force-pushed the remote-control branch 4 times, most recently from 5a9bbe0 to 194b73d Compare February 8, 2018 15:06
*
* \return true if remote control is enabled, false otherwise
*/
bool IsRemoteControlExists();

This comment was marked as spam.

@@ -34,7 +34,7 @@ class CIRTranslator
/*!
* \brief Loads Lircmap.xml/IRSSmap.xml
*/
void Load();
void Load(std::string irMapName);

This comment was marked as spam.

virtual void Disconnect() = 0;

/*!
* \brief Get the button readed from IR

This comment was marked as spam.

*
* \return A button code
*/
virtual unsigned short GetButton() const = 0;

This comment was marked as spam.

/*!
* \brief Get the time while a button was hold
*
* \return A hold time

This comment was marked as spam.

/*!
* \brief Set enabled status
*/
virtual void SetEnabled(bool) { }

This comment was marked as spam.

/*!
* \brief Get the in-use status
*/
virtual bool IsInUse() const { return false; }

This comment was marked as spam.

bool IsInUse() const override { return m_used; }
bool IsInitialized() const override { return m_bInitialized; }
void AddSendCommand(const std::string& command) override;
std::string GetMapFile() override { return "Lircmap.xml"; }

This comment was marked as spam.

/*!
* \brief Interface for remote control
*/
class IRemoteControl

This comment was marked as spam.

@garbear
Copy link
Member

garbear commented Feb 8, 2018

Really great job! All my comments are 1-liners

@garbear garbear closed this Feb 8, 2018
@garbear garbear reopened this Feb 8, 2018
/*!
* \brief Get the button readed from IR
*
* \return A button code

This comment was marked as spam.

@garbear
Copy link
Member

garbear commented Feb 8, 2018

A single request to make this refactoring even more stellar: Can you move XBIRRemote.h to your new remotes/ subdir? Maybe rename it to not include XB? thanks!

return LIRC_MAP_FILENAME;
}

void CRemoteControl::SetEnabled(bool bEnabled)

This comment was marked as spam.

This comment was marked as spam.

virtual bool IsInUse() const = 0;
};
}
}

This comment was marked as spam.

This comment was marked as spam.

/*!
* \brief Add a command to send to a bus
*/
virtual void AddSendCommand(const std::string&) = 0;

This comment was marked as spam.

@afedchin
Copy link
Member Author

afedchin commented Feb 9, 2018

if there is no more objection this will go tonight

@garbear
Copy link
Member

garbear commented Feb 9, 2018

go now

@afedchin afedchin merged commit e223aeb into xbmc:master Feb 9, 2018
@afedchin afedchin deleted the remote-control branch February 9, 2018 21:49
@MilhouseVH
Copy link
Contributor

This PR is causing a problem with remote input after resuming from suspend (on Linux, specifically LibreELEC).

Starting with build #0210, when device (eg. x86_64 Revo 3700 with USB MCE IR) is resumed from suspend Kodi no longer receives any remote control inputs.

I've built the latest master Kodi with and without this PR, and the version with this PR has the resume-from-suspend issue while the version without works perfectly after resuming.

Log without PR: http://ix.io/Gjw (works perfectly after resume)
Log with PR: http://ix.io/Gjw (no remote input seen after resume)

What seems to be missing in the "bad" log is the restart of lirc - this is what happens in the "good" log after resuming:

08:18:28.901 T:139900420427136  NOTICE: OnWake: Running resume jobs
08:18:28.901 T:139900420427136   DEBUG: ------ Window Deinit (DialogBusy.xml) ------
08:18:28.901 T:139900420427136  NOTICE: OnWake: Restarting lirc
08:18:28.903 T:139898102593280   DEBUG: Thread RemoteControl start, auto delete: false
08:18:28.903 T:139898102593280    INFO: LIRC Process: using: /run/lirc/lircd
08:18:28.904 T:139898102593280    INFO: LIRC Connect: successfully started
08:18:28.905 T:139898102593280   DEBUG: Thread RemoteControl 139898102593280 terminating

@afedchin
Copy link
Member Author

Log without PR: http://ix.io/Gjw (works perfectly after resume)
Log with PR: http://ix.io/Gjw (no remote input seen after resume)

the links are identical

@MilhouseVH
Copy link
Contributor

Sorry, fat fingers:

Log without PR: http://ix.io/Gjw (works perfectly after resume)
Log with PR: http://ix.io/GjB (no remote input seen after resume)

@afedchin
Copy link
Member Author

Should work now, see 5c9c318

@MilhouseVH
Copy link
Contributor

Thanks, 5c9c318 fixes the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Input Type: Cleanup non-breaking change which removes non-working or unmaintained functionality v18 Leia
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants