From 122fb5c5e9ecb6af6a44a7bf59b9d9defe2eac7b Mon Sep 17 00:00:00 2001 From: Neil Dorin Date: Tue, 11 Feb 2020 21:06:24 -0700 Subject: [PATCH 1/7] Adds appropriate condition check before printing console message --- Pepperdash Core/Pepperdash Core/Logging/Debug.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Pepperdash Core/Pepperdash Core/Logging/Debug.cs b/Pepperdash Core/Pepperdash Core/Logging/Debug.cs index 14c6b37..cf4d8df 100644 --- a/Pepperdash Core/Pepperdash Core/Logging/Debug.cs +++ b/Pepperdash Core/Pepperdash Core/Logging/Debug.cs @@ -91,7 +91,8 @@ namespace PepperDash.Core Level = context.Level; DoNotLoadOnNextBoot = context.DoNotLoadOnNextBoot; - CrestronConsole.PrintLine(string.Format("Program {0} will not load config after next boot. Use console command go:{0} to load the config manually", InitialParametersClass.ApplicationNumber)); + if(DoNotLoadOnNextBoot) + CrestronConsole.PrintLine(string.Format("Program {0} will not load config after next boot. Use console command go:{0} to load the config manually", InitialParametersClass.ApplicationNumber)); try { From c82376071187bc9b91445c408a0d1bacf6038754 Mon Sep 17 00:00:00 2001 From: Jason T Alborough Date: Fri, 21 Feb 2020 13:03:14 -0500 Subject: [PATCH 2/7] # GenericSecureTcpIpServer - Restarts the server whenever a faulty client connection occurs. This appears the only way to get the listener back after a faulty attempt. - Removes the condition on rearming the server.WaitForConnectionAsync callback. --- .../Comm/GenericSecureTcpIpServer.cs | 41 ++++++++++++------- .../Pepperdash Core/PepperDash_Core.csproj | 8 ++-- 2 files changed, 30 insertions(+), 19 deletions(-) diff --git a/Pepperdash Core/Pepperdash Core/Comm/GenericSecureTcpIpServer.cs b/Pepperdash Core/Pepperdash Core/Comm/GenericSecureTcpIpServer.cs index 6703551..f7e19cd 100644 --- a/Pepperdash Core/Pepperdash Core/Comm/GenericSecureTcpIpServer.cs +++ b/Pepperdash Core/Pepperdash Core/Comm/GenericSecureTcpIpServer.cs @@ -759,28 +759,39 @@ namespace PepperDash.Core else { Debug.Console(1, this, Debug.ErrorLogLevel.Error, "Client attempt faulty."); - if (!ServerStopped) - { - server.WaitForConnectionAsync(IPAddress.Any, SecureConnectCallback); - return; - } + + // JTA 2020-02-20 + // There is an issue where on a failed negotiation we need to stop and start the server. This should still leave connected client intact. + server.Stop(); + Listen(); + return; + } } catch (Exception ex) { Debug.Console(2, this, Debug.ErrorLogLevel.Error, "Error in Socket Status Connect Callback. Error: {0}", ex); } - //Debug.Console(1, this, Debug.ErrorLogLevel, "((((((Server State bitfield={0}; maxclient={1}; ServerStopped={2}))))))", - // server.State, - // MaxClients, - // ServerStopped); - if ((server.State & ServerState.SERVER_LISTENING) != ServerState.SERVER_LISTENING && MaxClients > 1 && !ServerStopped) - { - Debug.Console(1, this, Debug.ErrorLogLevel.Notice, "Waiting for next connection"); - server.WaitForConnectionAsync(IPAddress.Any, SecureConnectCallback); - } - } + server.WaitForConnectionAsync(IPAddress.Any, SecureConnectCallback); + + /* + Debug.Console(1, this, Debug.ErrorLogLevel.Error, "((((((Server State ={0} {3}; maxclient={1}; ServerStopped={2}))))))", + server.State, + MaxClients, + ServerStopped); + + // JTA 2020-02-21 Im was not clear on why this condition was here. I think the WaitForCOnnection should always rearm. + + if ((server.State & ServerState.SERVER_LISTENING) != ServerState.SERVER_LISTENING && MaxClients > 1 && !ServerStopped) + { + Debug.Console(1, this, Debug.ErrorLogLevel.Notice, "Waiting for next connection"); + server.WaitForConnectionAsync(IPAddress.Any, SecureConnectCallback); + + } + */ + + } #endregion diff --git a/Pepperdash Core/Pepperdash Core/PepperDash_Core.csproj b/Pepperdash Core/Pepperdash Core/PepperDash_Core.csproj index 1d98c97..92ce8c8 100644 --- a/Pepperdash Core/Pepperdash Core/PepperDash_Core.csproj +++ b/Pepperdash Core/Pepperdash Core/PepperDash_Core.csproj @@ -49,19 +49,19 @@ False - ..\..\..\..\..\..\..\ProgramData\Crestron\SDK\SimplSharpCustomAttributesInterface.dll + ..\..\..\..\..\..\..\..\ProgramData\Crestron\SDK\SimplSharpCustomAttributesInterface.dll False - ..\..\..\..\..\..\..\ProgramData\Crestron\SDK\SimplSharpHelperInterface.dll + ..\..\..\..\..\..\..\..\ProgramData\Crestron\SDK\SimplSharpHelperInterface.dll False - ..\..\..\..\..\..\..\ProgramData\Crestron\SDK\SimplSharpNewtonsoft.dll + ..\..\..\..\..\..\..\..\ProgramData\Crestron\SDK\SimplSharpNewtonsoft.dll False - ..\..\..\..\..\..\..\ProgramData\Crestron\SDK\SimplSharpReflectionInterface.dll + ..\..\..\..\..\..\..\..\ProgramData\Crestron\SDK\SimplSharpReflectionInterface.dll From c037d73e32ec3bac835d9207f7855f59fbbf85e8 Mon Sep 17 00:00:00 2001 From: Jason T Alborough Date: Sat, 22 Feb 2020 07:22:10 -0500 Subject: [PATCH 3/7] Adds logging of the SocketErrorCode when arming WaitForConnectionAsync. #8 --- .../Comm/GenericSecureTcpIpServer.cs | 55 +++++++++---------- 1 file changed, 26 insertions(+), 29 deletions(-) diff --git a/Pepperdash Core/Pepperdash Core/Comm/GenericSecureTcpIpServer.cs b/Pepperdash Core/Pepperdash Core/Comm/GenericSecureTcpIpServer.cs index f7e19cd..74ce8cb 100644 --- a/Pepperdash Core/Pepperdash Core/Comm/GenericSecureTcpIpServer.cs +++ b/Pepperdash Core/Pepperdash Core/Comm/GenericSecureTcpIpServer.cs @@ -418,11 +418,21 @@ namespace PepperDash.Core SecureServer.PortNumber = Port; } ServerStopped = false; - SecureServer.WaitForConnectionAsync(IPAddress.Any, SecureConnectCallback); - OnServerStateChange(SecureServer.State); - Debug.Console(1, this, Debug.ErrorLogLevel.Notice, "Secure Server Status: {0}, Socket Status: {1}", SecureServer.State, SecureServer.ServerSocketStatus); - ServerCCSection.Leave(); - + + // Start the listner + SocketErrorCodes status = SecureServer.WaitForConnectionAsync(IPAddress.Any, SecureConnectCallback); + if (status != SocketErrorCodes.SOCKET_OPERATION_PENDING) + { + Debug.Console(0, this, Debug.ErrorLogLevel.Error, "Error starting WaitForConnectionAsync {0}", status); + } + else + { + ServerStopped = false; + } + OnServerStateChange(SecureServer.State); + Debug.Console(1, this, Debug.ErrorLogLevel.Notice, "Secure Server Status: {0}, Socket Status: {1}", SecureServer.State, SecureServer.ServerSocketStatus); + ServerCCSection.Leave(); + } catch (Exception ex) { @@ -758,14 +768,7 @@ namespace PepperDash.Core } else { - Debug.Console(1, this, Debug.ErrorLogLevel.Error, "Client attempt faulty."); - - // JTA 2020-02-20 - // There is an issue where on a failed negotiation we need to stop and start the server. This should still leave connected client intact. - server.Stop(); - Listen(); - return; - + Debug.Console(1, this, Debug.ErrorLogLevel.Error, "Client attempt faulty."); } } catch (Exception ex) @@ -773,24 +776,18 @@ namespace PepperDash.Core Debug.Console(2, this, Debug.ErrorLogLevel.Error, "Error in Socket Status Connect Callback. Error: {0}", ex); } - server.WaitForConnectionAsync(IPAddress.Any, SecureConnectCallback); - - /* - Debug.Console(1, this, Debug.ErrorLogLevel.Error, "((((((Server State ={0} {3}; maxclient={1}; ServerStopped={2}))))))", - server.State, - MaxClients, - ServerStopped); - - // JTA 2020-02-21 Im was not clear on why this condition was here. I think the WaitForCOnnection should always rearm. - - if ((server.State & ServerState.SERVER_LISTENING) != ServerState.SERVER_LISTENING && MaxClients > 1 && !ServerStopped) + // Rearm the listner + SocketErrorCodes status = server.WaitForConnectionAsync(IPAddress.Any, SecureConnectCallback); + if (status != SocketErrorCodes.SOCKET_OPERATION_PENDING) { - Debug.Console(1, this, Debug.ErrorLogLevel.Notice, "Waiting for next connection"); - server.WaitForConnectionAsync(IPAddress.Any, SecureConnectCallback); - + Debug.Console(0, this, Debug.ErrorLogLevel.Error, "Socket status connect callback status {0}", status); + if (status == SocketErrorCodes.SOCKET_CONNECTION_IN_PROGRESS) + { + // There is an issue where on a failed negotiation we need to stop and start the server. This should still leave connected clients intact. + server.Stop(); + Listen(); + } } - */ - } #endregion From 31892b5996c89ea0708db5df5a4861dbb69a1730 Mon Sep 17 00:00:00 2001 From: Neil Dorin Date: Mon, 24 Feb 2020 10:26:23 -0700 Subject: [PATCH 4/7] Adds issue templates for bug and feature --- .github/ISSUE_TEMPLATE/bug_report.md | 34 +++++++++++++++++++++++ .github/ISSUE_TEMPLATE/feature_request.md | 21 ++++++++++++++ 2 files changed, 55 insertions(+) create mode 100644 .github/ISSUE_TEMPLATE/bug_report.md create mode 100644 .github/ISSUE_TEMPLATE/feature_request.md diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md new file mode 100644 index 0000000..c969d2a --- /dev/null +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -0,0 +1,34 @@ +--- +name: Bug report +about: Create a report to help us improve +title: "[BUG]-" +labels: bug +assignees: '' + +--- + +**Describe the bug** +A clear and concise description of what the bug is. + +**Stacktrace** + +Include a stack trace of the exception if possible. +``` +Paste stack trace here +``` + +**To Reproduce** +Steps to reproduce the behavior: +1. Go to '...' +2. Click on '....' +3. Scroll down to '....' +4. See error + +**Expected behavior** +A clear and concise description of what you expected to happen. + +**Screenshots** +If applicable, add screenshots to help explain your problem. + +**Additional context** +Add any other context about the problem here. diff --git a/.github/ISSUE_TEMPLATE/feature_request.md b/.github/ISSUE_TEMPLATE/feature_request.md new file mode 100644 index 0000000..0a854e7 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/feature_request.md @@ -0,0 +1,21 @@ +--- +name: Feature request +about: Suggest an idea for this project +title: "[FEATURE]-" +labels: enhancement +assignees: '' + +--- + +**Is your feature request related to a problem? Please describe.** +A clear and concise description of what the problem is. Ex. I'm always frustrated when [...] +If this is a request for support for a new device or type, be as specific as possible and include any pertinent manufacturer and model information. + +**Describe the solution you'd like** +A clear and concise description of what you want to happen. + +**Describe alternatives you've considered** +A clear and concise description of any alternative solutions or features you've considered. + +**Additional context** +Add any other context or screenshots about the feature request here. From b13f4251692707d518e1fd4ae8d7c7ae7ea32bb0 Mon Sep 17 00:00:00 2001 From: Neil Dorin Date: Tue, 25 Feb 2020 15:19:49 -0700 Subject: [PATCH 5/7] #11 adds semaphore to prevent Connect() from being called from S+ if a connection attempt is still in progress. --- .../Pepperdash Core/Comm/GenericSshClient.cs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/Pepperdash Core/Pepperdash Core/Comm/GenericSshClient.cs b/Pepperdash Core/Pepperdash Core/Comm/GenericSshClient.cs index 46892c7..ef57ae0 100644 --- a/Pepperdash Core/Pepperdash Core/Comm/GenericSshClient.cs +++ b/Pepperdash Core/Pepperdash Core/Comm/GenericSshClient.cs @@ -62,6 +62,8 @@ namespace PepperDash.Core get { return ClientStatus == SocketStatus.SOCKET_STATUS_CONNECTED; } } + private bool IsConnecting = false; + /// /// S+ helper for IsConnected /// @@ -184,6 +186,13 @@ namespace PepperDash.Core /// public void Connect() { + if (IsConnecting) + { + Debug.Console(0, this, "Connection attempt in progress. Exiting Connect()"); + return; + } + + IsConnecting = true; ConnectEnabled = true; Debug.Console(1, this, "attempting connect"); @@ -236,6 +245,7 @@ namespace PepperDash.Core //TheStream.ErrorOccurred += TheStream_ErrorOccurred; Debug.Console(1, this, "Connected"); ClientStatus = SocketStatus.SOCKET_STATUS_CONNECTED; + IsConnecting = false; return; // Success will not pass here } catch (SshConnectionException e) @@ -291,6 +301,7 @@ namespace PepperDash.Core if (Client != null) { + IsConnecting = false; Client.Disconnect(); Client = null; ClientStatus = status; @@ -303,6 +314,7 @@ namespace PepperDash.Core /// void HandleConnectionFailure() { + KillClient(SocketStatus.SOCKET_STATUS_CONNECT_FAILED); Debug.Console(2, this, "Client nulled due to connection failure. AutoReconnect: {0}, ConnectEnabled: {1}", AutoReconnect, ConnectEnabled); From 8e626fd28022d67211b6d5d41bcaa56cb0c41f41 Mon Sep 17 00:00:00 2001 From: Neil Dorin Date: Thu, 27 Feb 2020 12:38:15 -0700 Subject: [PATCH 6/7] Create main.yml --- .github/workflows/main.yml | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 .github/workflows/main.yml diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml new file mode 100644 index 0000000..aeaa527 --- /dev/null +++ b/.github/workflows/main.yml @@ -0,0 +1,20 @@ +name: Build Branch + +on: + push: + branches: + - feature/* + - bugfix/* + +jobs: + build: + name: Build + runs-on: self-hosted + steps: + - name: Trigger Jenkins Job + uses: appleboy/jenkins-action@0.0.1 + with: + url: "http://example.com" + user: "example" + token: ${{ secrets.TOKEN }} + job: "PepperDash Core Branch Builds" From a81e7fa951d67ee30b5611e19bf34e62486aef8b Mon Sep 17 00:00:00 2001 From: Neil Dorin Date: Thu, 27 Feb 2020 12:48:20 -0700 Subject: [PATCH 7/7] Update main.yml Adds url and user values --- .github/workflows/main.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index aeaa527..f2a4149 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -14,7 +14,7 @@ jobs: - name: Trigger Jenkins Job uses: appleboy/jenkins-action@0.0.1 with: - url: "http://example.com" - user: "example" + url: "http://localhost:8080" + user: "ndorin" token: ${{ secrets.TOKEN }} job: "PepperDash Core Branch Builds"