openlp-core team mailing list archive

[Phill <phill.ridout@xxxxxxxxx>]

Review: Needs Fixing

A few issues / questions. See in line.

Diff comments:

> 
> === modified file 'openlp/core/projectors/editform.py'
> --- openlp/core/projectors/editform.py	2017-12-29 09:15:48 +0000
> +++ openlp/core/projectors/editform.py	2018-03-24 08:24:22 +0000
> @@ -58,10 +58,15 @@
>          # IP Address
>          self.ip_label = QtWidgets.QLabel(edit_projector_dialog)
>          self.ip_label.setObjectName('projector_edit_ip_label')
> -        self.ip_text = QtWidgets.QLineEdit(edit_projector_dialog)
> -        self.ip_text.setObjectName('projector_edit_ip_text')
> +        self.ip_text_edit = QtWidgets.QLineEdit(edit_projector_dialog)
> +        self.ip_text_edit.setObjectName('projector_edit_ip_text')
> +        self.ip_text_show = QtWidgets.QLabel(edit_projector_dialog)

append _label to the name please

> +        self.ip_text_show.setObjectName('projector_show_ip_text')
>          self.dialog_layout.addWidget(self.ip_label, 0, 0)
> -        self.dialog_layout.addWidget(self.ip_text, 0, 1)
> +        # For new projector, use edit widget
> +        self.dialog_layout.addWidget(self.ip_text_edit, 0, 1)
> +        # For edit projector, use show widget
> +        self.dialog_layout.addWidget(self.ip_text_show, 0, 1)
>          # Port number
>          self.port_label = QtWidgets.QLabel(edit_projector_dialog)
>          self.port_label.setObjectName('projector_edit_ip_label')
> @@ -187,7 +198,8 @@
>                                                                                              record=record.id)))
>              valid = False
>              return
> -        adx = self.ip_text.text()
> +        if self.new_projector:

'adx' will not be defined if this is False

> +            adx = self.ip_text_edit.text()
>          valid = verify_ip_address(adx)
>          if valid:
>              ip = self.projectordb.get_projector_by_ip(adx)
> 
> === modified file 'openlp/core/projectors/pjlink.py'
> --- openlp/core/projectors/pjlink.py	2018-02-11 11:42:13 +0000
> +++ openlp/core/projectors/pjlink.py	2018-03-24 08:24:22 +0000
> @@ -973,45 +975,60 @@
>              ip = self.ip
>          log.debug('({ip}) get_data(ip="{ip_in}" buffer="{buff}"'.format(ip=self.entry.name, ip_in=ip, buff=buff))
>          # NOTE: Class2 has changed to some values being UTF-8
> -        data_in = decode(buff, 'utf-8')
> +        if type(buff) is bytes:

I think isinstance(buff, bytes) is preferred.

> +            data_in = decode(buff, 'utf-8')
> +        else:
> +            data_in = buff
>          data = data_in.strip()
> +        # log.debug('({ip}) get_data() data_in="{data}"'.format(ip=self.entry.name, data=data_in))

Is this temporary?

>          # Initial packet checks
>          if (len(data) < 7):
>              self._trash_buffer(msg='get_data(): Invalid packet - length')
>              return self.receive_data_signal()
>          elif len(data) > self.max_size:
> -            self._trash_buffer(msg='get_data(): Invalid packet - too long')
> +            self._trash_buffer(msg='get_data(): Invalid packet - too long ({length} bytes)'.format(length=len(data)))
>              return self.receive_data_signal()
>          elif not data.startswith(PJLINK_PREFIX):
>              self._trash_buffer(msg='get_data(): Invalid packet - PJLink prefix missing')
>              return self.receive_data_signal()
> -        elif '=' not in data:
> +        elif data[6] != '=':
>              self._trash_buffer(msg='get_data(): Invalid reply - Does not have "="')
>              return self.receive_data_signal()
>          log.debug('({ip}) get_data(): Checking new data "{data}"'.format(ip=self.entry.name, data=data))
>          header, data = data.split('=')
> +        log.debug('({ip}) get_data() header="{header}" data="{data}"'.format(ip=self.entry.name,
> +                                                                             header=header, data=data))
>          # At this point, the header should contain:
>          #   "PVCCCC"
>          #   Where:
>          #       P = PJLINK_PREFIX
>          #       V = PJLink class or version
>          #       C = PJLink command
> +        version, cmd = header[1], header[2:].upper()
> +        log.debug('({ip}) get_data() version="{version}" cmd="{cmd}"'.format(ip=self.entry.name,
> +                                                                             version=version, cmd=cmd))
> +        '''

Whats happening with this code here to .....

>          try:
>              version, cmd = header[1], header[2:].upper()
> +            log.debug('({ip}) get_data() version="{version}" cmd="{cmd}"'.format(ip=self.entry.name,
> +                                                                                 version=version, cmd=cmd))
>          except ValueError as e:
>              self.change_status(E_INVALID_DATA)
>              log.warning('({ip}) get_data(): Received data: "{data}"'.format(ip=self.entry.name, data=data_in))
>              self._trash_buffer('get_data(): Expected header + command + data')
>              return self.receive_data_signal()
> +        '''

.... here

>          if cmd not in PJLINK_VALID_CMD:
> -            log.warning('({ip}) get_data(): Invalid packet - unknown command "{data}"'.format(ip=self.entry.name,
> +            self._trash_buffer('get_data(): Invalid packet - unknown command "{data}"'.format(ip=self.entry.name,
>                                                                                                data=cmd))
> -            self._trash_buffer(msg='get_data(): Unknown command "{data}"'.format(data=cmd))
> -            return self.receive_data_signal()
> -        if int(self.pjlink_class) < int(version):
> +            return self.receive_data_signal()
> +        elif version not in PJLINK_VALID_CMD[cmd]['version']:
> +            self._trash_buffer(msg='get_data() Command reply version does not match a valid command version')
> +            return self.receive_data_signal()
> +        elif int(self.pjlink_class) < int(version):
>              log.warning('({ip}) get_data(): Projector returned class reply higher '
>                          'than projector stated class'.format(ip=self.entry.name))
> -        self.process_command(cmd, data)
> +        self.process_command(cmd, data, *args, **kwargs)
>          return self.receive_data_signal()
>  
>      @QtCore.pyqtSlot(QtNetwork.QAbstractSocket.SocketError)


-- 
https://code.launchpad.net/~alisonken1/openlp/pjlink2-q/+merge/342022
Your team OpenLP Core is subscribed to branch lp:openlp.