Re: [Openlp-core] [Merge] lp:~alisonken1/openlp/pjlink2-q into lp:openlp
Review: Approve -- https://code.launchpad.net/~alisonken1/openlp/pjlink2-q/+merge/343669 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
Re: [Openlp-core] [Merge] lp:~alisonken1/openlp/pjlink2-q into lp:openlp
Review: Approve Looks good to me. Thanks! -- https://code.launchpad.net/~alisonken1/openlp/pjlink2-q/+merge/343669 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
Re: [Openlp-core] [Merge] lp:~alisonken1/openlp/pjlink2-q into lp:openlp
Review: Needs Fixing A few issues / questions. See in line. Diff comments: > > === modified file 'openlp/core/projectors/editform.py' > --- openlp/core/projectors/editform.py2017-12-29 09:15:48 + > +++ openlp/core/projectors/editform.py2018-03-24 08:24:22 + > @@ -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 + > +++ openlp/core/projectors/pjlink.py 2018-03-24 08:24:22 + > @@ -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: > # "PV" > # 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: >
Re: [Openlp-core] [Merge] lp:~alisonken1/openlp/pjlink2-q into lp:openlp
Review: Approve Looks OK but projectors is not my strong point. -- https://code.launchpad.net/~alisonken1/openlp/pjlink2-q/+merge/342022 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
Re: [Openlp-core] [Merge] lp:~alisonken1/openlp/pjlink2-q into lp:openlp
which fix is clashing? I tried a merge from trunk and it merged fine. -- https://code.launchpad.net/~alisonken1/openlp/pjlink2-q/+merge/341563 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
Re: [Openlp-core] [Merge] lp:~alisonken1/openlp/pjlink2-q into lp:openlp
> Sorry will clash with one of my fixes which has been merged > > See question >> 'ACKN': {'version': ['2', ], >> + 'default': '2', >Why do we have a missing slot? 2 new commands in PJLink are only valid for version 2 and have no version 1 equivalent. Missing slot is to maintain compatibility with version checker. >> -def process_clss(self, data): >> +def process_clss(self, data, *args, **kwargs): >why args Some commands take host,port arguments but not all of them do. Since there is a common entry point, need to keep availability of consuming extra args/kwargs without causing exception due to extra parameters. (Yes, there should be no args passed only keyword args, but better safe than sorry) -- https://code.launchpad.net/~alisonken1/openlp/pjlink2-q/+merge/341563 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
Re: [Openlp-core] [Merge] lp:~alisonken1/openlp/pjlink2-q into lp:openlp
Review: Needs Fixing Sorry will clash with one of my fixes which has been merged See question Diff comments: > > === modified file 'openlp/core/projectors/constants.py' > --- openlp/core/projectors/constants.py 2018-01-03 00:35:14 + > +++ openlp/core/projectors/constants.py 2018-03-17 17:48:23 + > @@ -154,110 +154,137 @@ > S_INFO > ] > > -# NOTE: Changed format to account for some commands are both class 1 and 2 > +# NOTE: Changed format to account for some commands are both class 1 and 2. > +# Make sure the sequence of 'version' is lowest-to-highest. > PJLINK_VALID_CMD = { > 'ACKN': {'version': ['2', ], > + 'default': '2', Why do we have a missing slot? > 'description': translate('OpenLP.PJLinkConstants', >'Acknowledge a PJLink SRCH command - > returns MAC address.') > }, > 'AVMT': {'version': ['1', ], > + 'default': '1', > 'description': translate('OpenLP.PJLinkConstants', >'Blank/unblank video and/or mute > audio.') > }, > 'CLSS': {'version': ['1', ], > + 'default': '1', > 'description': translate('OpenLP.PJLinkConstants', >'Query projector PJLink class > support.') > }, > 'ERST': {'version': ['1', '2'], > + 'default': '1', > 'description': translate('OpenLP.PJLinkConstants', >'Query error status from projector. ' >'Returns > fan/lamp/temp/cover/filter/other error status.') > }, > 'FILT': {'version': ['2', ], > + 'default': '1', > 'description': translate('OpenLP.PJLinkConstants', >'Query number of hours on filter.') > }, > 'FREZ': {'version': ['2', ], > + 'default': '1', > 'description': translate('OpenLP.PJLinkConstants', >'Freeze or unfreeze current image > being projected.') > }, > 'INF1': {'version': ['1', ], > + 'default': '1', > 'description': translate('OpenLP.PJLinkConstants', >'Query projector manufacturer name.') > }, > 'INF2': {'version': ['1', ], > + 'default': '1', > 'description': translate('OpenLP.PJLinkConstants', >'Query projector product name.') > }, > 'INFO': {'version': ['1', ], > + 'default': '1', > 'description': translate('OpenLP.PJLinkConstants', >'Query projector for other information > set by manufacturer.') > }, > 'INNM': {'version': ['2', ], > + 'default': '2', > 'description': translate('OpenLP.PJLinkConstants', >'Query specified input source name') > }, > 'INPT': {'version': ['1', ], > + 'default': '1', > 'description': translate('OpenLP.PJLinkConstants', >'Switch to specified video source.') > }, > 'INST': {'version': ['1', ], > + 'default': '1', > 'description': translate('OpenLP.PJLinkConstants', >'Query available input sources.') > }, > 'IRES': {'version:': ['2', ], > + 'default': '2', > 'description': translate('OpenLP.PJLinkConstants', >'Query current input resolution.') > }, > 'LAMP': {'version': ['1', ], > + 'default': '1', > 'description': translate('OpenLP.PJLinkConstants', >'Query lamp time and on/off status. > Multiple lamps supported.') > }, > 'LKUP': {'version': ['2', ], > + 'default': '2', > 'description': translate('OpenLP.PJLinkConstants', >'UDP Status - Projector is now > available on network. Includes MAC address.') > }, > 'MVOL': {'version': ['2', ], > + 'default': '1', > 'description': translate('OpenLP.PJLinkConstants', >'Adjust microphone volume by 1 step.') > }, > 'NAME': {'version': ['1', ], > + 'default': '1', > 'description': translate('OpenLP.PJLinkConstants', >'Query customer-set projector name.') > }, > 'PJLINK': {'version': ['1', ], > + 'default': '1', > 'description':