eonpatapon / mpDris2

MPRIS V2.1 support for mpd
GNU General Public License v3.0
214 stars 46 forks source link

Check new_status before updating _status #128

Closed ferdnyc closed 4 years ago

ferdnyc commented 4 years ago

This bug report came in via Fedora's Abrt reporting system. The pertinent details:

Truncated backtrace:
mpDris2:734:_update_properties:TypeError: 'bool' object is not subscriptable

Traceback (most recent call last):
  File "/usr/bin/mpDris2", line 454, in timer_callback
    self._update_properties(force=False)
  File "/usr/bin/mpDris2", line 734, in _update_properties
    if old_status['state'] != new_status['state']:
TypeError: 'bool' object is not subscriptable

Local variables in innermost frame:
self: <__main__.MPDWrapper object at 0x7f365b81ba50>
force: False
old_status: False
old_position: 228.723
old_time: 1580885354
new_status: {'volume': '40', 'repeat': '0', 'random': '1', 'single': '0', 'consume': '0', 'playlist': '2', 'playlistlength': '13080', 'mixrampdb': '0.000000', 'state': 'pause', 'song': '10067', 'songid': '10068', 'time': '229:240', 'elapsed': '228.723', 'bitrate': '320', 'duration': '239.647', 'audio': '44100:24:2', 'nextsong': '8546', 'nextsongid': '8547'}
new_time: 1580885355
new_position: 228.723

This seems to happen because _update_properties sets self._status to the value of new_status before checking that new_status is valid. Then, on the next iteration, old_status is set to the value of self._status, which contains merely False.

This PR updates the order of operations so that self._status is updated with the value of new_status only if new_status does not evaluate to False.

ferdnyc commented 4 years ago

Bah!! I just realized I should've moved that debug log after the new_status check, since now it's actually logging the old status. I'll submit another PR. :laughing: