openlp-core team mailing list archive

[Tim Bentley <tim.bentley@xxxxxxxxx>]

Review: Needs Fixing

See inline

Diff comments:

> 
> === modified file 'openlp/core/projectors/pjlink.py'
> --- openlp/core/projectors/pjlink.py	2017-11-24 19:08:23 +0000
> +++ openlp/core/projectors/pjlink.py	2017-12-05 00:54:42 +0000
> @@ -608,6 +656,11 @@
>              self.projectorReceivedData.disconnect(self._send_command)
>          except TypeError:
>              pass
> +        try:
> +            self.readyRead.disconnect(self.get_socket)  # Set in process_pjlink
> +        except TypeError:
> +            pass
> +

No space

>          self.disconnect_from_host()
>          self.deleteLater()
>          self.i_am_running = False
> @@ -848,32 +907,43 @@
>              log.debug('({ip}) get_socket(): No data available (-1)'.format(ip=self.ip))
>              return self.receive_data_signal()
>          self.socket_timer.stop()
> -        return self.get_data(buff=read, ip=self.ip)
> +        self.get_data(buff=read, ip=self.ip)
> +        return self.receive_data_signal()
>  
> -    def get_data(self, buff, ip):
> +    def get_data(self, buff, ip=None):
>          """
>          Process received data
>  
>          :param buff:    Data to process.
>          :param ip:      (optional) Destination IP.
>          """
> +        ip = self.ip if ip is None else ip

this is confusing can this not be done on the caller?

>          log.debug("({ip}) get_data(ip='{ip_in}' buffer='{buff}'".format(ip=self.ip, ip_in=ip, buff=buff))
>          # NOTE: Class2 has changed to some values being UTF-8
>          data_in = decode(buff, 'utf-8')
>          data = data_in.strip()
> -        if (len(data) < 7) or (not data.startswith(PJLINK_PREFIX)):
> -            return self._trash_buffer(msg='get_data(): Invalid packet - length or prefix')
> +        # Initial packet checks
> +        if (len(data) < 7):
> +            return self._trash_buffer(msg="get_data(): Invalid packet - length")
>          elif len(data) > self.max_size:
> -            return self._trash_buffer(msg='get_data(): Invalid packet - too long')
> +            return self._trash_buffer(msg="get_data(): Invalid packet - too long")
> +        elif not data.startswith(PJLINK_PREFIX):
> +            return self._trash_buffer(msg="get_data(): Invalid packet - PJLink prefix missing")
>          elif '=' not in data:
> -            return self._trash_buffer(msg='get_data(): Invalid packet does not have equal')
> -        log.debug('({ip}) get_data(): Checking new data "{data}"'.format(ip=self.ip, data=data))
> +            return self._trash_buffer(msg="get_data(): Invalid packet - Does not have '='")
> +        log.debug("({ip}) get_data(): Checking new data '{data}'".format(ip=self.ip, data=data))
>          header, data = data.split('=')
> +        # At this point, the header should contain:
> +        #   "PVCCCC"
> +        #   Where:
> +        #       P = PJLINK_PREFIX
> +        #       V = PJLink class or version
> +        #       C = PJLink command
>          try:
> -            version, cmd = header[1], header[2:]
> +            version, cmd = header[1], header[2:].upper()
>          except ValueError as e:
>              self.change_status(E_INVALID_DATA)
> -            log.warning('({ip}) get_data(): Received data: "{data}"'.format(ip=self.ip, data=data_in.strip()))
> +            log.warning('({ip}) get_data(): Received data: "{data}"'.format(ip=self.ip, data=data_in))
>              return self._trash_buffer('get_data(): Expected header + command + data')
>          if cmd not in PJLINK_VALID_CMD:
>              log.warning('({ip}) get_data(): Invalid packet - unknown command "{data}"'.format(ip=self.ip, data=cmd))


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