PR 1 ...#208
Closed
FitzerIRL wants to merge 0 commit into
Closed
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the core framework to improve comms/logging compatibility, including Python 3.13’s removal of telnetlib, and adds more flexibility in selecting which DUT console session to use.
Changes:
- Add a Python 3.13+
telnetlibimport fallback intelnetClass.py. - Update
testControlto pick a console session based on an “enabled” console config (with default fallback). - Update
logModuleto set UTF-8 encoding on log file handlers and adjust handler cleanup.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| framework/core/testControl.py | Adds logic to select a non-default console session and guards telnetlib import. |
| framework/core/logModule.py | Uses UTF-8 file encodings and improves logger handler cleanup behavior. |
| framework/core/commandModules/telnetClass.py | Adds a minimal telnetlib compatibility layer for Python 3.13+. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
+40
to
+47
| class Telnet: | ||
| def __init__(self, host=None, port=23, timeout=socket._GLOBAL_DEFAULT_TIMEOUT): | ||
| self.host = host | ||
| self.port = port | ||
| self.timeout = timeout | ||
| self.sock = None | ||
| if host: | ||
| self.open(host, port, timeout) |
Comment on lines
+60
to
+61
| if self.sock: | ||
| self.sock.send(buffer) |
Comment on lines
+36
to
+40
| try: | ||
| import telnetlib | ||
| except ImportError: | ||
| # telnetlib was removed in Python 3.13, this is handled in telnetClass.py | ||
| telnetlib = None |
Comment on lines
+164
to
+170
| dut_config = self.slotInfo.config.get("devices", [{}])[0].get("dut", {}) | ||
| consoles_config = dut_config.get("consoles", []) | ||
|
|
||
| # Find the enabled console | ||
| for console_item in consoles_config: | ||
| for name, config in console_item.items(): | ||
| if config.get("enabled", False): |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the core framework to improve compatibility (notably around Python 3.13 changes) and make device console selection more flexible by respecting enabled flags in console configuration.
Changes:
- Add optional-import handling for
telnetlib(Python 3.13) and for heavy optional dependencies (capture,webpageController). - Respect
enabled: falsein console configuration and improve console-session selection/fallback behavior. - Logging improvements: avoid handler-removal edge cases and force UTF-8 encoding for log files.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| framework/core/testControl.py | Select console session based on enabled console config; adds optional telnetlib import guard. |
| framework/core/logModule.py | Safer handler cleanup and UTF-8 log file encoding. |
| framework/core/deviceManager.py | Skip disabled consoles and add fallback console selection when requested console is unavailable. |
| framework/core/commandModules/telnetClass.py | Adds a Python 3.13 telnetlib fallback layer. |
| framework/core/commandModules/sshConsole.py | Whitespace-only cleanup. |
| framework/core/commandModules/serialClass.py | Minor robustness tweak when handling empty reads; whitespace cleanup. |
| framework/core/init.py | Makes capture and webpageController optional exports when dependencies aren’t installed. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
+228
to
+236
| console = self.consoles.get(consoleName) | ||
| # If requested console was disabled/missing, fall back to first enabled console | ||
| if console is None and self.consoles: | ||
| fallback = next(iter(self.consoles)) | ||
| self.log.info("Console '{}' not available, falling back to '{}'".format(consoleName, fallback)) | ||
| console = self.consoles[fallback] | ||
| if console is None: | ||
| self.log.error("No consoles available (all disabled?)") | ||
| return None |
Comment on lines
+70
to
+74
| # Respect 'enabled: false' in console config — skip disabled consoles | ||
| if config.get("enabled") is False: | ||
| self.type = None | ||
| self.session = None | ||
| return |
| except Exception as e: | ||
| self.log.warn(f"Failed to detect enabled console, using default: {e}") | ||
|
|
||
| self.session = self.dut.getConsoleSession(console_name) |
Comment on lines
+36
to
+40
| try: | ||
| import telnetlib | ||
| except ImportError: | ||
| # telnetlib was removed in Python 3.13, this is handled in telnetClass.py | ||
| telnetlib = None |
Comment on lines
+35
to
+47
| try: | ||
| from . capture import capture | ||
| except ImportError: | ||
| capture = None # cv2/pytesseract/PIL not installed (e.g. Docker) | ||
| from . commonRemote import commonRemoteClass | ||
| from . deviceManager import deviceManager | ||
| from . logModule import logModule | ||
| from . logModule import DEBUG, INFO, WARNING, ERROR, CRITICAL | ||
| from . testControl import testController | ||
| from . webpageController import webpageController | ||
| try: | ||
| from . webpageController import webpageController | ||
| except ImportError: | ||
| webpageController = None # selenium not installed (e.g. Docker) |
Comment on lines
+40
to
+102
| class Telnet: | ||
| def __init__(self, host=None, port=23, timeout=socket._GLOBAL_DEFAULT_TIMEOUT): | ||
| self.host = host | ||
| self.port = port | ||
| self.timeout = timeout | ||
| self.sock = None | ||
| if host: | ||
| self.open(host, port, timeout) | ||
|
|
||
| def open(self, host, port=23, timeout=socket._GLOBAL_DEFAULT_TIMEOUT): | ||
| self.host = host | ||
| self.port = port | ||
| self.sock = socket.create_connection((host, port), timeout) | ||
|
|
||
| def close(self): | ||
| if self.sock: | ||
| self.sock.close() | ||
| self.sock = None | ||
|
|
||
| def write(self, buffer): | ||
| if self.sock: | ||
| self.sock.send(buffer) | ||
|
|
||
| def read_until(self, match, timeout=None): | ||
| if not self.sock: | ||
| return b'' | ||
|
|
||
| buffer = b'' | ||
| start_time = time.time() | ||
| while True: | ||
| if timeout and (time.time() - start_time) > timeout: | ||
| break | ||
| try: | ||
| data = self.sock.recv(1024) | ||
| if not data: | ||
| break | ||
| buffer += data | ||
| if match in buffer: | ||
| break | ||
| except socket.timeout: | ||
| break | ||
| except Exception: | ||
| break | ||
| return buffer | ||
|
|
||
| def read_all(self): | ||
| if not self.sock: | ||
| return b'' | ||
| buffer = b'' | ||
| try: | ||
| while True: | ||
| data = self.sock.recv(1024) | ||
| if not data: | ||
| break | ||
| buffer += data | ||
| except Exception: | ||
| pass | ||
| return buffer | ||
|
|
||
| # Create a mock telnetlib module | ||
| class telnetlib: | ||
| Telnet = Telnet | ||
|
|
Contributor
There was a problem hiding this comment.
Copilot wasn't able to review any files in this pull request.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Updates for compatibility and flexibility of comms