diff --git a/FabLabKasse/UI/FAUcardPaymentDialogCode.py b/FabLabKasse/UI/FAUcardPaymentDialogCode.py index 017a8a0..a20e7d7 100644 --- a/FabLabKasse/UI/FAUcardPaymentDialogCode.py +++ b/FabLabKasse/UI/FAUcardPaymentDialogCode.py @@ -21,9 +21,6 @@ class FAUcardPaymentDialog(QtWidgets.QDialog, Ui_FAUcardPaymentDialog): # Signal to tell the FAUcardThread to send response acknowledge to the MagnaBox + optional cancel of the process response_ack = QtCore.Signal(bool) - # Signal to tell the payment handler to terminate the thread - request_termination = QtCore.Signal() - def __init__(self, parent, amount, shopping_backend): """ Initializes the FAUcardPaymentDialog. It sets it's member variables and sets up the GUI, including @@ -52,9 +49,6 @@ def __init__(self, parent, amount, shopping_backend): self.counter = 0 self.thread_aborted = False self.thread_broken = False - self.timer_terminate = QtCore.QTimer() - self.timer_terminate.timeout.connect(self.request_thread_termination) - self.timer_terminate.setSingleShot(True) self.status = Status.initializing # Start a timer to periodically update the GUI (show life sign) @@ -117,8 +111,6 @@ def update_gui(self, response): ), "Thread response code is not Status or Info!" self.thread_broken = False - if self.timer_terminate.isActive(): - self.timer_terminate.stop() if isinstance(response[0], Info): # Abort if checking the last transaction failed @@ -240,20 +232,10 @@ def try_reject(self): If the thread is finished tell the user the process is aborting and process the final abortion in 2 seconds """ if self.thread_aborted is not True: - QtWidgets.QMessageBox.warning( - None, - "FAUCard Zahlung", - "Abbrechen gerade nicht möglich.\nBitte warten Sie, bis das Programm an der nächst \ - möglichen Stelle abbricht. Beachte bitte, das falls dein Geld schon abgebucht \ - wurde, es nicht automatisch rückabgewickelt wird.", - ) self.label_status.setText("Warte auf Möglichkeit zum Abbruch") - if not self.timer_terminate.isActive(): - self.timer_terminate.start(60000) else: self.label_status.setText("Breche Bezahlung ab.") self.pushButton_abbrechen.hide() - self.timer_terminate.stop() QtCore.QTimer.singleShot(2000, self.reject_final) @QtCore.Slot() @@ -262,21 +244,3 @@ def reject_final(self): Final rejection of the Payment """ QtWidgets.QDialog.reject(self) - - @QtCore.Slot() - def request_thread_termination(self): - """ - Sets the thread_broken flag to let the user terminate the thread if necessary. - """ - self.thread_broken = True - terminate = QtWidgets.QMessageBox.question( - None, - "FAUCard Zahlung", - "Willst du den Thread terminieren? Wenn du dir nicht sicher bist, antworte mit nein.", - QtWidgets.QMessageBox.Yes, - QtWidgets.QMessageBox.No, - ) - if terminate == QtWidgets.QMessageBox.Yes: - logging.error("FAUcardPayment: thread termination was requested") - self.request_termination.emit() - QtCore.QTimer.singleShot(500, self.reject_final) diff --git a/FabLabKasse/faucardPayment/FAUcardPaymentThread.py b/FabLabKasse/faucardPayment/FAUcardPaymentThread.py index 520cf44..2cc519a 100644 --- a/FabLabKasse/faucardPayment/FAUcardPaymentThread.py +++ b/FabLabKasse/faucardPayment/FAUcardPaymentThread.py @@ -10,6 +10,7 @@ from qtpy import QtCore, QtWidgets from decimal import Decimal from datetime import datetime +import time from .faucardStates import Status, Info from .MagPosLog import MagPosLog @@ -98,6 +99,7 @@ def __init__( self.card_number: Optional[int] = None self.old_balance: Optional[int] = None self.new_balance: Optional[int] = None + self.run_finished = False assert amount > 0 self.amount = amount @@ -108,7 +110,6 @@ def __init__( self.ack = False self.sleep_counter = 0 self.last_sleep = 0 - self.should_finish_log = True self.timestamp_payed: Optional[datetime] = None @@ -152,16 +153,6 @@ def set_ack(self, cf: bool) -> None: self.ack = True self.cancel = cf - # set if thread should finish logfile with booking entry - @QtCore.Slot(bool) - def set_should_finish_log(self, val: bool) -> None: - """ - Sets the finish logfile flag - :param val: Contains information if the thread should finish the log file - :type val: bool - """ - self.should_finish_log = val - @QtCore.Slot() def user_abortion(self) -> None: """sets cancel flag to cancel the payment""" @@ -199,8 +190,7 @@ def sleep(self, seconds: int) -> None: return counter = self.sleep_counter - # BUG? what happens if sleep_counter accidentally (timing glitch, ...) becomes larger than seconds*10? - while self.sleep_counter is not seconds * 10: + while self.sleep_counter < seconds * 10: if self.cancel == True: raise self.UserAbortionError("sleep") if counter == self.sleep_counter: @@ -234,13 +224,33 @@ def quit(self) -> None: @QtCore.Slot() def run(self) -> None: + """ + Run payment. See _run(). + This function blocks until the payment has finished or an Exception was thrown. + Afterwards, run_finished is set to True. + + This function may call the event-loop during the payment operation ("voluntary preemption"). + """ + try: + self._run() + finally: + # Make sure that in all cases, + try: + # - the status is written to the database. + if self.log is not None: + self.log.set_status(self.status, self.info) + finally: + # - and run_finished is set to True + self.run_finished = True + + def _run(self) -> None: """ Billing routine for FAU-Card payment. Runs following steps: 1. Check last transaction result 2. Initialize log class and connection to MagnaBox 3. Read the card the user puts on the reader 4. Decrease the amount from card balance - (Optional: 5. finish log file) + (Note: The MagPosLog log entry is NOT marked as "booking done" here, because at the end of this function the booking in the cash register is not yet done. After the booking in the cash register was done, faucardPayment.faucard.finish_log() needs to be called to do that.) After each step the routine waits till the GUI sends a positive feedback by set_ack slot. The GUI can send a cancel flag with the response_ack signal and is able to quit the process after every step. """ @@ -290,10 +300,6 @@ def run(self) -> None: self.log.set_timestamp_payed(self.timestamp_payed) logging.debug("FAUcardThread: New Balance: {}".format(self.new_balance)) - # 5. finish log entry - if self.should_finish_log is True: - self.finish_log() - except (magpos.ResponseError, self.ConnectionError) as e: logging.error("FAUcardThread: {}".format(e)) self.terminate() @@ -496,18 +502,21 @@ def _read_card( def _decrease_balance(self) -> int: """ Decreases card balance by self.amount_cents by following steps: + 0. If user aborted: Quit by raising UserAbortionError. 1. Try to decrease balance -> a: Successful - 2. Continue with step 5 - -> b: Connection was lost? + Continue with step 5 + -> aa: Payment was not executed (user removed card) + Continue with step 0. + -> b: Connection was lost (i.e., we don't know yet if the payment was successful) 2. Try to reestablish connection, ignore connection relevant errors 3. Check if the payment you tried has been successfully executed -> a: was successfully executed: 4. Continue with step 5 -> b: the payment was not executed: - 4: Go back to step 1 + 4: Go back to step 0 -> c: it is unclear if the payment was executed or not: - 4. Quit the process by raising an Exception + 4. Quit by raising an Exception 5. Check if payment was correct and log it :return: New balance on success. Otherwise an exception will be raised. :rtype: int @@ -533,17 +542,18 @@ def _decrease_balance(self) -> int: decrease_result = None - # 1. Try to decrease balance + # 0./1. Check for user abortion and try to decrease balance while retry: retry = False self.set_cancel_button_enabled.emit( True ) # User must be able to abort if he decides to try: + logging.debug("FAUcard: 0. Check if user aborted") self.check_user_abort( "decreasing balance: User Aborted" ) # Will only be executed if decrease command has not yet been executed - logging.debug("FAUcard: Trying to decrease balance") + logging.debug("FAUcard: 1. Trying to decrease balance") decrease_result = self.pos.decrease_card_balance_and_token( self.amount_cents, self.card_number ) @@ -551,8 +561,9 @@ def _decrease_balance(self) -> int: # Catch ResponseError if not Card on Reader and retry except magpos.ResponseError as e: if e.code is magpos.codes.NO_CARD: - logging.info("FAUcard: No card, retrying...") + logging.info("FAUcard: No card, retrying... (1.aa)") self.pos.response_ack() + # -> Back to 0. retry = True continue else: @@ -569,11 +580,11 @@ def _decrease_balance(self) -> int: self.response_ready.emit([Info.con_error]) lost = True - # Abortion of the process not allowed - # self.set_cancel_button_enabled.emit(False) - # Clear cancel Flag if user tried to abort: no abortion allowed after this tep - QtCore.QCoreApplication.processEvents() - self.cancel = False + # From now on (all lines in this function below this comment), abortion of the process not allowed. + # The only exception is when we are sure that the payment did not succeed (the amount was not deducted from the card), in which case we can go back to step 0 and retry the payment. + # + # "Abortion not allowed" is implemented by NOT calling _wait_for_ack() of check_user_abort() in any line below this comment. + # The user can still click on the cancel button and therefore set self.cancel=True, but this will be ignored until we get to a point at which canceling is allowed. # if connection lost (1.b) if lost: @@ -582,7 +593,7 @@ def _decrease_balance(self) -> int: while lost: lost = False - logging.debug("FAUcard: 2 Try to reastablish connect") + logging.debug("FAUcard: 2 Try to reestablish connect") try: # release serial port self.pos.close() @@ -605,6 +616,9 @@ def _decrease_balance(self) -> int: IOError, ): lost = True + # wait a short time to avoid flooding the harddrive with error messages when the connection is permanently lost + # we don't use self.sleep() here because we do NOT want to process cancel-events + time.sleep(10) self.info = Info.con_back self.response_ready.emit([Info.con_back]) diff --git a/FabLabKasse/faucardPayment/faucard.py b/FabLabKasse/faucardPayment/faucard.py index 477807b..8831134 100644 --- a/FabLabKasse/faucardPayment/faucard.py +++ b/FabLabKasse/faucardPayment/faucard.py @@ -11,6 +11,7 @@ from datetime import datetime from decimal import Decimal from qtpy import QtCore, QtWidgets, QtGui +import logging from .MagPosLog import MagPosLog from ..UI.FAUcardPaymentDialogCode import FAUcardPaymentDialog @@ -39,16 +40,24 @@ def __init__(self, parent, amount, shopping_backend): self.dialog = FAUcardPaymentDialog( parent=parent, amount=self.amount, shopping_backend=self.shopping_backend ) - self.dialog.request_termination.connect( - self.threadTerminationRequested, type=QtCore.Qt.DirectConnection - ) self.worker = FAUcardThread( dialog=self.dialog, amount=self.amount, thread=self.thread ) def executePayment(self): """ - Starts the Payment-process and Dialog + Runs the Payment-process: Open dialog, wait for payment to complete (or to fail), close dialog, clean up. + + Expected usage when the user requests "Start FAUCard payment": + 1. Call this function + 2. If it returned True: + The payment has succeeded. + a. Write the corresponding booking into the accounting / cash register database. + b. call finish_log(), which then notes in the FAUCard MagPosLog logging that the payment has been fully booked. + Note that if a) crashes, then we can see this in the MagPos database status. + Else: + The payment has failed, go back to the shopping-cart view so that the user can restart the payment. + :return: True on success, False otherwise :rtype: bool """ @@ -56,15 +65,27 @@ def executePayment(self): self.thread.start() self.dialog.show() - # Execute the GUI + # Execute the GUI and wait until the dialog has closed. success = self.dialog.exec_() - if success == QtWidgets.QDialog.Accepted: - return True - else: - # Wait for thread to finish cleanup - self.thread.wait(1000) - return False + # Now, the payment routine (FAUCardThread.run()) has returned (or raised an Exception). + # double-check this: + while not self.worker.run_finished: + # TODO: rewrite the code so that it is obvious that this can not happen. + # --> FAUCardThread itself should subclass QThread and override run(). + logging.error( + "Payment routine did not finish (deadlock?), waiting... This should never happen." + ) + time.sleep(10) + + # Stop thread's event loop, it is no longer needed + self.thread.quit() + while not self.thread.wait(10000): + logging.error( + "thread refuses to stop, waiting... This should never happen except when e.g. the system is under extreme CPU load." + ) + + return success == QtWidgets.QDialog.Accepted def getPaidAmount(self): """ @@ -83,54 +104,6 @@ def getWantReceipt(self): # always output a receipt due to legal requirements return True - def finishLogEntry(self): - """ - Completes the log entry in the MagPosLog and closes all open threads and dialogs - """ - if self.thread.isRunning(): - QtCore.QMetaObject.invokeMethod( - self.worker, - "set_ack", - QtCore.Qt.QueuedConnection, - QtCore.Q_ARG(bool, False), - ) - self.close() - - @QtCore.Slot() - def threadTerminationRequested(self): - """ - Terminates the self.thread if requested. - """ - self.thread.wait(100) - self.thread.quit() - if not self.thread.wait(1000): - self.thread.terminate() - - def close(self): - """ - Quits self.thread, closes self.dialog - """ - self.dialog.close() - if self.thread.isRunning(): - QtCore.QMetaObject.invokeMethod( - self.worker, - "set_should_finish_log", - QtCore.Qt.QueuedConnection, - QtCore.Q_ARG(bool, False), - ) - QtCore.QMetaObject.invokeMethod( - self.worker, - "set_ack", - QtCore.Qt.QueuedConnection, - QtCore.Q_ARG(bool, False), - ) - self.thread.wait(100) - if not self.thread.isRunning(): - return - self.thread.quit() - if not self.thread.wait(1000): - self.thread.terminate() - def check_last_transaction(): """ diff --git a/FabLabKasse/shopping/payment_methods.py b/FabLabKasse/shopping/payment_methods.py index 16317ed..edad59f 100644 --- a/FabLabKasse/shopping/payment_methods.py +++ b/FabLabKasse/shopping/payment_methods.py @@ -353,8 +353,6 @@ def _show_dialog(self): self.amount_paid = pay_func.getPaidAmount() self.amount_returned = 0 - pay_func.close() - def _end_of_payment(self): """ Is required to complete the MagPosLog logging after the payment routine.