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

Code rework for Kodi plugin #116

Closed
volumio opened this issue Dec 18, 2017 · 6 comments · Fixed by #145
Closed

Code rework for Kodi plugin #116

volumio opened this issue Dec 18, 2017 · 6 comments · Fixed by #145

Comments

@volumio
Copy link
Owner

volumio commented Dec 18, 2017

@Saiyato

Before deploying the Kodi plugin there are few issues that need to be solved:

  • The UIConfig does not get loaded:
    ERROR LOADING JSON Error: ENOENT: no such file or directory, open '/data/plugins/miscellanea/Kodi/UIConfig.json'
  • Tvservice is not found:
    Dec 18 22:42:37 Volumio kodi-standalone[29879]: /usr/bin/kodi: 46: /usr/bin/kodi: tvservice: not found

use always the full path: /opt/vc/bin/tvservice

  • Not sure if it's related, but:
    oreCommandRouter::Close All Modals sent
    Dec 18 22:42:38 Volumio kodi-standalone[29879]: *** Error in `/usr/lib/arm-linux-gnueabihf/kodi/kodi_v7.bin': munmap_chunk(): invalid pointer: 0x02dda4b4 ***
    Dec 18 22:42:38 Volumio kodi-standalone[29879]: * failed to open vchiq instance
    Dec 18 22:42:38 Volumio kodi-standalone[29879]: Aborted (core dumped)
@Saiyato
Copy link
Contributor

Saiyato commented Dec 19, 2017

I just patched the tvservice not found error, it needed a symlink in the install script.

I couldn't reproduce the JSON error, do you have more info for me? How can I test that?

The last error is caused by a memory shortage, you need a reboot after installation (/boot/config.txt is changed)

@Saiyato
Copy link
Contributor

Saiyato commented Dec 19, 2017

Never mind, I see what happened... you removed the zip file; the problem is with the directory name I used, I use a capital K and uploaded a non-capital k directory.

What do you think? Capital K or not? I have no real preference, just used the capital because it's a name.

Maybe even make it a naming convention, all lower or something like that.

@volumio
Copy link
Owner Author

volumio commented Dec 19, 2017

  • Use all lowercase (so no capital K)
  • Avoid as much as possible to write to SD Card (so don't make the symlink if possible)
  • If the user needs to restart you must tell him, so use a popup to notify, see alsa controller plugin to see how to do it
  • I removed the zip because we should always produce from source, this way we can keep code consistent

@Saiyato
Copy link
Contributor

Saiyato commented Dec 19, 2017

Check! Will (try to) patch the directory name tomorrow.

The symlink is because the file is read from /usr/bin/ as opposed to /opt/vc/bin/, it's not really necessary, but the error is ugly imho. It might just work if I move the tvservice binary to /usr/bin/ not sure what that would mean for other applications (I think nothing, but it's just a theory).

I will also read into the alsa controller plugin... I know how to create the pop-up, just not after installation (from install.sh), don't know if the onInstall method works right now.

@volumio
Copy link
Owner Author

volumio commented Dec 19, 2017

  • We can add an alias for tv service, I think its useful. Can you send a pr to the build repo? (look in volumioconfig.sh when we create the aliases), we can take the chance of adding the command to nopasswd as well
  • We can extend the rest api service to include a call from which you can broadcast a popup, we need to be creative with this ;) so you can invoke it from the sh

@Saiyato
Copy link
Contributor

Saiyato commented Dec 20, 2017

Added the PR for tv service, not sure if nopasswd is required. Heck, I don't even fully understand what it does, I believe it detects attached screens, but apart from that...

The pop-up in the API would help yes, haven't had the time to read into the alsa controller, but I presume that pop-up doesn't come from sh.

Testing the new directory name now first, hopefully I can push the version tonight.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants