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

LMS Error reporting and code cleanup #1564

Merged
merged 4 commits into from
Apr 18, 2014
Merged

Conversation

NovaXeros
Copy link
Contributor

  • Improved Logitech Media Server error reporting.
  • Improved overall code.
  • Changed configuration title.

Found that my LMS code was reporting errors to the logger even upon success. Cleaned that up, and also streamlined the original code a little.

Also changed the title in the config options. Referring to the system as LMS is alright when you know what LMS stands for, but using it as the title is a bit daft for people who may not know what LMS is at first glance. Similarly, using 'Squeezebox' is also a bit redundant based on that being an old disused name.

Even upon successful rescan, the code would throw a keyerror back. Not certain why, but LMS sends some really weird unicode JSON back as its result, unlike XBMC, so having response[0]['result'] will always throw back a keyerror.

Removing the key, however, and just asking the return to find 'result' works fine.
Squeezebox is an old unused name, and LMS isn't very self-explanitory.
@rembo10
Copy link
Owner

rembo10 commented Apr 18, 2014

can you just double check that i merged this in right? there was a merge conflict and i'm not 100% sure. just notifiers.py

cheers!

@rembo10
Copy link
Owner

rembo10 commented Apr 18, 2014

merged into the develop branch, btw

@rembo10 rembo10 merged commit c96b043 into rembo10:master Apr 18, 2014
@evtk
Copy link

evtk commented Apr 21, 2014

There is still a glitch in the error reporting. Command to LMS was successfully given (LMS did a rescan) but log headphones shows otherwise:

2014-04-21 16:53:43 WARNING Error sending rescan request to LMS
2014-04-21 16:53:42 INFO    Sending library rescan command to LMS @ http://localhost:9002

@NovaXeros NovaXeros deleted the patch-1 branch April 22, 2014 10:11
@NovaXeros
Copy link
Contributor Author

Confirmed that LMS is still reporting an error, though not to the extent it used to; not sure how I missed it when I tested my code. Must have been late.

As long as LMS is correctly updating, pretty much all is well. I'll fix that bit of code as soon as I can, but at least it still works for now. Cheers for letting me know.

And rembo10, I checked the merge as you asked me and didn't notice any problems with it, so I'll keep my eyes on it and see if anything develops.

@evtk
Copy link

evtk commented Dec 15, 2014

Just checking in to report that the wrong error report is still not fixed.

@NovaXeros
Copy link
Contributor Author

Thanks for the comment EvtK. RL caught up with me this year and I totally forgot to address this issue. Should now be patched finally in #2048.

@evtk
Copy link

evtk commented Dec 16, 2014

cool.. thanks!

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

Successfully merging this pull request may close these issues.

4 participants