Skip to content
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

Refactor library to support multiple drivers. #62

Merged
merged 3 commits into from
Mar 12, 2024

Conversation

spudwebb
Copy link

I created a JsonConverter called ZWJSSJsonConverter so that the Controller, ZWaveNode and Endpoint classes are automatically instantiated with a driver property when deserialized from JSON.

I made all the other changes you listed in #61 (comment)

I successfully tested the library in client mode with 2 drivers.

For the self-hosted mode, I guess it will need some additional changes because in Server.Start() the first thing it does is kill any existing server.psi process, so that won't work if multiple servers are created. I will let you handle this part as I'm not using the self-hosted mode.

@marcus-j-davies
Copy link
Member

marcus-j-davies commented Feb 21, 2024

Woah! Nice work @spudwebb

I'll give it the eye over the next few days as well as fixing up host mode with the changes.
I am considering about dropping the reference application, as it's causing too many delays in getting v4 released.

Question: How much have you tested the lib, I wouldn't imagine every method has been tested (or has it)?
I would like to think there isn't anything of concern at this point.

Before a release, I will review any critical schema updates to bring it up to date with the server (its only 2 behind)

Thanks for the contributions you have made on the project, it's much appreciated!

@marcus-j-davies marcus-j-davies linked an issue Feb 21, 2024 that may be closed by this pull request
@spudwebb
Copy link
Author

We have a beta version of a new ZWave driver for HomeSeer based on this library that we are currently testing internally. This new driver uses a good chunk of the library but not every methods.
The things that we do not have tested at all are:

  • Use of the library in host mode
  • Virtual nodes
  • Node/Controller/Route statistics
  • MethodFactory

@georgeinva2004
Copy link

If it helps, I have tested Virtual Nodes and are consuming the statistics for display purposes. Not doing anything more than just displaying stats. Beautiful piece of work folks!!!!

@marcus-j-davies
Copy link
Member

Thanks @georgeinva2004

With all testing that seems to have taken place - I will try my upmost to get a release window in (along with this PR)

Thanks both 👍

@marcus-j-davies
Copy link
Member

@spudwebb

My solution to host mode is as follows..

  1. A copy of the server (server.psi) is made, but named along with the configured port (server.50001.psi)
  2. This copy is what will be run.
  3. but not before killing (and deleting) any instances of this copy
 internal void Start(string SerialPort, ZWaveOptions Config, int WSPort)
 {
     string ProcessName = string.Format("server.{0}.psi", WSPort);

     Process[] Zombies = Process.GetProcessesByName(ProcessName);
     foreach(Process Zombie in Zombies)
     {
         Zombie.Kill();
         File.Delete(ProcessName);
     }

     if (!File.Exists("server.psi"))
     {
         throw new FileNotFoundException("No Platform Snapshot Image (server.psi) found");
     }

     File.Copy("server.psi",ProcessName, true);
     ...
     ProcessStartInfo.FileName = ProcessName
     Process.Start()
     
}

This should allow mulitple servers if starting up multiple Self Hosted instances (I will check for ports already in use)

@marcus-j-davies
Copy link
Member

marcus-j-davies commented Mar 8, 2024

@spudwebb
Sorry - got pulled away - I'll look to merge this over the weekend along with the host changes

@marcus-j-davies marcus-j-davies merged commit 3fe7bdc into zwave-js:4.0.0 Mar 12, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants