-
Notifications
You must be signed in to change notification settings - Fork 35
add eknitter ip handling #727
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
21f0d9c
eb22202
ab69af7
98b4980
e7209d3
26b411e
c79c8b7
0ab3cf0
a4b4925
3421c34
2dfdb4c
9d92dfd
0ddad61
27c4adb
2395d30
9c197eb
441d3cd
791b142
03566c0
2a3ff95
2ee3738
e6def79
a73f58c
fcc7202
d9fee94
01f8e91
e80ec4b
9348c83
73a27cf
a76c279
1aa6794
b6da180
20bff05
5b6a7f1
9662cf2
a092ada
81dc059
44344d6
13a1416
9458342
1dd8685
fb59d12
c488c86
39f1d90
0a4b204
d1f1935
7f2bba4
b90bc31
0a63bfe
5148bc5
894c1e8
9d2bd6b
ec5636b
1a8ee29
c190c10
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,3 +6,4 @@ types-pyserial==3.5.0 | |
types-mock | ||
types-Pillow==10 | ||
mypy==1.9.0 | ||
esptool==3.3.3 |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -16,7 +16,7 @@ Function .onInit | |||||
;Do not use InstallDir at all so we can detect empty $InstDir! | ||||||
${If} $InstDir == "" ; /D not used | ||||||
${If} $MultiUser.InstallMode == "AllUsers" | ||||||
StrCpy $InstDir "C:\%{app_name}-v%{version}" | ||||||
StrCpy $InstDir "$%HOMEDRIVE%\%{app_name}-v%{version}" | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Check environment variable syntax in NSIS script The current syntax Consider using one of these alternatives: - StrCpy $InstDir "$%HOMEDRIVE%\%{app_name}-v%{version}"
+ StrCpy $InstDir "$HOMEDRIVE\%{app_name}-v%{version}" Or: - StrCpy $InstDir "$%HOMEDRIVE%\%{app_name}-v%{version}"
+ StrCpy $InstDir "%HOMEDRIVE%\%{app_name}-v%{version}" 📝 Committable suggestion
Suggested change
|
||||||
${Else} | ||||||
StrCpy $InstDir "$LOCALAPPDATA\%{app_name}" | ||||||
${EndIf} | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,7 @@ | |
# Copyright 2014 Sebastian Oliva, Christian Obersteiner, | ||
# Andreas Müller, Christian Gerbrandt | ||
# https://github.com/AllYarnsAreBeautiful/ayab-desktop | ||
# Copyright 2024 Marcus Hoose (eKnitter.com) | ||
"""Provides a graphical interface for users to operate AYAB.""" | ||
|
||
from __future__ import annotations | ||
|
@@ -25,6 +26,7 @@ | |
|
||
from PySide6.QtWidgets import QMainWindow | ||
from PySide6.QtCore import QCoreApplication | ||
from PySide6.QtCore import QEvent | ||
|
||
from .main_gui import Ui_MainWindow | ||
from .gui_fsm import gui_fsm | ||
|
@@ -58,6 +60,11 @@ class GuiMain(QMainWindow): | |
components from `menu_gui.ui`. | ||
""" | ||
|
||
def closeEvent(self, event: QEvent) -> None: | ||
print("close") | ||
self.engine.close() | ||
event.accept() | ||
|
||
def __init__(self, app_context: AppContext): | ||
super().__init__() | ||
self.app_context = app_context | ||
|
@@ -68,7 +75,7 @@ def __init__(self, app_context: AppContext): | |
|
||
# create UI | ||
self.ui = Ui_MainWindow() | ||
self.ui.setupUi(self) | ||
self.ui.setupUi(self) # type: ignore | ||
|
||
# add modular components | ||
self.menu = Menu(self) | ||
|
@@ -131,6 +138,7 @@ def __set_prefs(self) -> None: | |
|
||
def __quit(self) -> None: | ||
logging.debug("Quitting") | ||
self.engine.close() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification 🛠️ Refactor suggestion Based on the search results, I can now generate the final response: Add error handling to engine.close() and avoid duplicate cleanup The verification reveals two locations where
This creates a potential issue where:
Recommendations:
def closeEvent(self, event):
print("close")
- self.engine.close()
+ try:
+ self.engine.close()
+ except Exception as e:
+ logging.error(f"Error during cleanup: {e}")
event.accept()
def __quit(self) -> None:
logging.debug("Quitting")
- self.engine.close() # Remove duplicate cleanup
instance = QCoreApplication.instance()
if instance is not None:
instance.quit()
sys.exit() 🔗 Analysis chainVerify cleanup sequence and add error handling The cleanup sequence in
Let's verify the cleanup sequence: Consider this implementation: def __quit(self) -> None:
logging.debug("Quitting")
- self.engine.close()
+ try:
+ self.engine.close()
+ except Exception as e:
+ logging.error(f"Error during cleanup: {e}")
+
instance = QCoreApplication.instance()
if instance is not None:
instance.quit()
- sys.exit()
+ # Allow event loop one last iteration to complete cleanup
+ QCoreApplication.processEvents()
+ sys.exit(0) 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Search for other cleanup handlers that might conflict
rg -A 5 "def (close|cleanup|shutdown|dispose|exit|quit)"
# Search for engine.close() usage
ast-grep --pattern 'engine.close()'
Length of output: 3821 |
||
instance = QCoreApplication.instance() | ||
if instance is not None: | ||
instance.quit() | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,200 @@ | ||
# -*- coding: utf-8 -*- | ||
# This file is part of AYAB. | ||
# | ||
# AYAB is free software: you can redistribute it and/or modify | ||
# it under the terms of the GNU General Public License as published by | ||
# the Free Software Foundation, either version 3 of the License, or | ||
# (at your option) any later version. | ||
# | ||
# AYAB is distributed in the hope that it will be useful, | ||
# but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
# GNU General Public License for more details. | ||
# | ||
# You should have received a copy of the GNU General Public License | ||
# along with AYAB. If not, see <http://www.gnu.org/licenses/>. | ||
# | ||
# Copyright 2024 Marcus Hoose (eKnitter.com) | ||
"""Handles the IP communication protocol. | ||
|
||
This module handles IP communication, currently works in a synchronous way. | ||
""" | ||
|
||
from .communication import * | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Avoid wildcard imports for clarity and maintainability. Using Apply this diff to replace the wildcard import with explicit imports: -from .communication import *
+from .communication import Communication, Token, Machine This change ensures that
🧰 Tools🪛 Ruff23-23: (F403) |
||
from ..machine import * | ||
|
||
import socket | ||
|
||
import logging | ||
import pprint | ||
from time import sleep | ||
|
||
# Port for TCP | ||
remotePort = 12346 | ||
|
||
class CommunicationIP(Communication): | ||
def __init__(self) -> None: | ||
logging.basicConfig(level=logging.DEBUG) | ||
self.logger = logging.getLogger(type(self).__name__) | ||
self.__tarAddressPort: tuple[str | None, int] = ("255.255.255.255", 12345) | ||
self.__sockTCP: None | socket.socket = None | ||
# socket.socket(family=socket.AF_INET, type=socket.SOCK_STREAM) | ||
self.rx_msg_list = list() | ||
self.version = 6 | ||
|
||
def __del__(self) -> None: | ||
self.close_socket() | ||
|
||
def is_open(self) -> bool: | ||
if self.__sockTCP is not None: | ||
return True | ||
else: | ||
return False | ||
|
||
def open_serial(self, portname: str | None = None) -> bool: | ||
print("open: " , portname) | ||
return self.open_tcp(portname) | ||
|
||
def close_serial(self) -> None: | ||
return None | ||
|
||
def open_tcp(self, pPortname: str | None = None) -> bool: | ||
try: | ||
self.__portname = pPortname | ||
self.__tarAddressPort = (self.__portname, remotePort) | ||
self.__sockTCP = socket.socket(family=socket.AF_INET, type=socket.SOCK_STREAM) | ||
self.__sockTCP.settimeout(10.0) | ||
self.__sockTCP.connect(self.__tarAddressPort) | ||
self.__sockTCP.settimeout(0.0) | ||
self.__sockTCP.setblocking(False) | ||
self.logger.info("Open TCP Socket successful") | ||
return True | ||
except: | ||
self.logger.info("Open TCP Socket faild") | ||
return False | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
def close_socket(self) -> None: | ||
if self.__sockTCP is not None: | ||
try: | ||
self.__sockTCP.close() | ||
del self.__sockTCP | ||
self.logger.info("Closing TCP Socket successful.") | ||
except: | ||
self.logger.warning("Closing TCP Socket failed. (mem Leak?)") | ||
self.__sockTCP = None | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
def send(self, data: bytearray) -> None: | ||
if self.__sockTCP is not None: | ||
try: | ||
self.__sockTCP.send(bytes(data)) | ||
# self.logger.info("SEND b'"+data+"'") | ||
sleep(0.5) | ||
except Exception as e: | ||
if hasattr(e, 'message'): | ||
self.logger.exception(e.message) | ||
else: | ||
self.logger.exception("Connection...Error") | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
self.close_socket() | ||
|
||
# NB this method must be the same for all API versions | ||
def req_info(self) -> None: | ||
self.send(bytearray([Token.reqInfo.value,self.version])) | ||
|
||
def req_test_API6(self) -> None: | ||
self.send(bytearray([Token.reqTest.value])) | ||
|
||
def req_start_API6(self, start_needle: int, stop_needle: int, | ||
continuous_reporting: bool, disable_hardware_beep: bool) -> None: | ||
"""Send a start message to the device.""" | ||
data = bytearray() | ||
data.append(Token.reqStart.value) | ||
data.append(start_needle) | ||
data.append(stop_needle) | ||
data.append( | ||
1 * continuous_reporting + | ||
2 * (not disable_hardware_beep)) | ||
hash = 0 | ||
hash = add_crc(hash, data) | ||
data.append(hash) | ||
self.send(data) | ||
|
||
def req_init_API6(self, machine: Machine) -> None: | ||
"""Send a start message to the device.""" | ||
data = bytearray() | ||
data.append(Token.reqInit.value) | ||
data.append(machine.value) | ||
hash = 0 | ||
hash = add_crc(hash, data) | ||
data.append(hash) | ||
self.send(data) | ||
|
||
def cnf_line_API6(self, line_number: int , color: int, flags: int, line_data: bytes) -> None: | ||
"""Send a line of data via the serial port. | ||
|
||
Send a line of data to the serial port. All arguments are mandatory. | ||
The data sent here is parsed by the Arduino controller which sets the | ||
knitting needles accordingly. | ||
|
||
Args: | ||
line_number (int): The line number to be sent. | ||
color (int): The yarn color to be sent. | ||
flags (int): The flags sent to the controller. | ||
line_data (bytes): The bytearray to be sent to needles. | ||
""" | ||
data = bytearray() | ||
data.append(Token.cnfLine.value) | ||
data.append(line_number) | ||
data.append(color) | ||
data.append(flags) | ||
data.extend(line_data) | ||
hash = 0 | ||
hash = add_crc(hash, data) | ||
data.append(hash) | ||
self.send(data) | ||
|
||
def update_API6(self) -> tuple[bytes | None, Token, int]: | ||
"""Read data from serial and parse as SLIP packet.""" | ||
return self.parse_API6(self.read_API6()) | ||
|
||
def parse_API6(self, msg: bytes | None) -> tuple[bytes | None, Token, int]: | ||
if msg is None: | ||
return None, Token.none, 0 | ||
# else | ||
for t in list(Token): | ||
if msg[0] == t.value: | ||
return msg, t, msg[1] | ||
# fallthrough | ||
self.logger.debug("unknown message: ") # drop crlf | ||
pp = pprint.PrettyPrinter(indent=4) | ||
pp.pprint(msg[1: -1].decode()) | ||
return msg, Token.unknown, 0 | ||
|
||
def read_API6(self) -> bytes | None: | ||
"""Read data from serial as SLIP packet.""" | ||
if self.__sockTCP is not None: | ||
try: | ||
data = self.__sockTCP.recv(1024) | ||
except BlockingIOError: | ||
data = bytes() | ||
except Exception as e: | ||
if hasattr(e, 'message'): | ||
self.logger.exception(e.message) | ||
else: | ||
self.logger.exception("Connection...Error") | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
self.close_socket() | ||
self.open_tcp(self.__portname) | ||
data = bytes() | ||
|
||
if len(data) > 0: | ||
self.rx_msg_list.append(data) | ||
if len(self.rx_msg_list) > 0: | ||
return self.rx_msg_list.pop(0) # FIFO | ||
# else | ||
return None | ||
|
||
def write_API6(self, cmd: bytes | bytearray) -> None: | ||
# SLIP protocol, no CRC8 | ||
if self.__ser: | ||
self.__ser.write(bytes(cmd)) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
<RCC> | ||
<qresource prefix="/"> | ||
<file>eKnitter.png</file> | ||
</qresource> | ||
</RCC> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
<RCC> | ||
<qresource prefix="/"> | ||
<file>eKnitter-reversed.png</file> | ||
</qresource> | ||
</RCC> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix the empty link in the issue reporting instructions.
The link for "opening a new issue" is currently empty, which breaks the functionality of the document and has been flagged by static analysis.
📝 Committable suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
28-28: No empty links
null
(MD042, no-empty-links)