openlp-core team mailing list archive

[Phill <phill.ridout@xxxxxxxxx>]

Review: Needs Fixing

Sorry, reviewed a previous version.

Diff comments:

> 
> === modified file 'openlp/core/projectors/pjlink.py'
> --- openlp/core/projectors/pjlink.py	2018-04-20 06:04:43 +0000
> +++ openlp/core/projectors/pjlink.py	2018-04-28 07:52:45 +0000
> @@ -108,22 +111,20 @@
>          if read_size < 0:
>              log.warning('(UDP) No data (-1)')
>              return
> -        if read_size < 1:
> +        elif read_size < 1:

Think I miscommunicated what I was trying to say. Hopefully: https://bin.snyman.info/mmmz6tdw makes my point

>              log.warning('(UDP) get_datagram() called when pending data size is 0')
>              return
> -        data, peer_address, peer_port = self.readDatagram(self.pendingDatagramSize())
> +        elif read_size > PJLINK_MAX_PACKET:
> +            log.warning('(UDP) UDP Packet too large ({size} bytes)- ignoring'.format(size=read_size))
> +            return
> +        data_in, peer_host, peer_port = self.readDatagram(read_size)
> +        data = data_in.decode('utf-8') if isinstance(data_in, bytes) else data_in
>          log.debug('(UDP) {size} bytes received from {adx} on port {port}'.format(size=len(data),
> -                                                                                 adx=peer_address,
> -                                                                                 port=peer_port))
> +                                                                                 adx=peer_host.toString(),
> +                                                                                 port=self.port))
>          log.debug('(UDP) packet "{data}"'.format(data=data))
> -        # Send to appropriate instance to process packet
> -        log.debug('(UDP) Checking projector list for ip {host} to process'.format(host=peer_address))
> -        for projector in self.projector_list:
> -            if peer_address == projector.ip:
> -                # Dispatch packet to appropriate remote instance
> -                log.debug('(UDP) Dispatching packet to {host}'.format(host=projector.entry.name))
> -                return projector.get_data(buff=data, ip=peer_address, host=peer_address, port=peer_port)
> -        log.warning('(UDP) Could not find projector with ip {ip} to process packet'.format(ip=peer_address))
> +        log.debug('(UDP) Sending data_received signal to projectors')
> +        self.data_received.emit(peer_host, self.localPort(), data)
>          return
>  
>      def search_start(self):
> @@ -644,20 +641,18 @@
>                  log.warning('({ip}) NOT saving serial number'.format(ip=self.entry.name))
>                  self.serial_no_received = data
>  
> -    def process_srch(self, data, host, port):
> +    def process_srch(self, data):
>          """
>          Process the SRCH command.
>  
>          SRCH is processed by terminals so we ignore any packet.
>  
>          :param data: Data in packet
> -        :param host: IP address of sending host
> -        :param port: Port received on
>          """
> -        log.warning('(UDP) SRCH packet received from {host} - ignoring'.format(host=host))
> +        log.warning("({ip}) I don't do SRCH packets - ignoring".format(ip=self.entry.ip))

Perhaps  '{ip} does not support SRCH packets - ignoring. would be better.

>          return
>  
> -    def process_sver(self, data, *args, **kwargs):
> +    def process_sver(self, data):
>          """
>          Software version of projector
>          """
> @@ -1181,7 +1185,7 @@
>          try:
>              self.readyRead.disconnect(self.get_socket)
>          except TypeError:
> -            pass
> +            log.debug('({ip}) disconnect_from_host(): TypeError detected'.format(ip=self.entry.name))

log.exception would be better as it world also log the traceback

>          log.debug('({ip}) disconnect_from_host() '
>                    'Current status {data}'.format(ip=self.entry.name, data=self._get_status(self.status_connect)[0]))
>          if abort:
> 
> === modified file 'openlp/core/projectors/tab.py'
> --- openlp/core/projectors/tab.py	2017-12-29 09:15:48 +0000
> +++ openlp/core/projectors/tab.py	2018-04-28 07:52:45 +0000
> @@ -89,6 +89,10 @@
>          self.connect_box_layout.addRow(self.dialog_type_label, self.dialog_type_combo_box)
>          self.left_layout.addStretch()
>          self.dialog_type_combo_box.activated.connect(self.on_dialog_type_combo_box_changed)
> +        # Connect on LKUP packet received (PJLink v2+ only)
> +        self.connect_on_linkup = QtWidgets.QCheckBox(self.connect_box)

rename to connect_on_linkup_check_box please

> +        self.connect_on_linkup.setObjectName('connect_on_linkup')
> +        self.connect_box_layout.addRow(self.connect_on_linkup)
>  
>      def retranslateUi(self):
>          """


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