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

Bug Fixes. and Build script changes #46

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

EliteScientist
Copy link

Fixed several crashes
Added new methods from the latest openzwave
Modified build script to build with Visual Studio

there are still some issues with the build script where it doesn't include the dlls in the jar file when building under windows. I wasn't going to create the pull request for that file until' it was complete but github wouldn't allow me to cherry pick commits.

Michael Rochelle added 4 commits September 18, 2016 20:02
…error

Fixed issue where Manager and Options instance was not cleared after calling destroy
Added functions:
 - requestNodeNeighborUpdate
 - deleteAllReturnRoutes
 - assignReturnRoute
 - addNode
 - removeNode
 - removeFailedNode
 - hasNodeFailed
 - replaceFailedNode
@mcicolella
Copy link

@zgmnkv when will this PR be integrated?

@mcicolella
Copy link

@zgmnkv any news?

@MrNeuronix MrNeuronix mentioned this pull request Nov 13, 2013
Closed
@ffbenedetti
Copy link

@zgmnkv any news on getting this merged??

*.iml
*.ipr
*.iws
/.nb-gradle/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need a new line at the end of file

@@ -133,6 +133,14 @@ model {
}
}
clang(Clang) {
}

visualCpp(VisualCpp)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 spaced should be used instead of tabs here and in other places

@@ -200,9 +208,11 @@ model {
}
cpp(CppSourceSet) {
lib library: "hidApi", linkage: "api"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

redundant whitespaces

source {
if (targetPlatform.operatingSystem.windows) {
if (targetPlatform.operatingSystem.windows)
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

curly brace should be on the same line as if

@@ -253,7 +263,7 @@ model {
srcDir "$openZWaveDir/cpp/src"
srcDir "$openZWaveDir/cpp/src/command_classes"
srcDir "$openZWaveDir/cpp/src/value_classes"
srcDir "$openZWaveDir/cpp/src/platform"
srcDir "$openZWaveDir/cpp/src/platform/"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't think that trailing slash is necessary

@@ -338,6 +351,13 @@ model {
if (toolChain in Gcc) {
linker.args "-static", "-lsetupapi"
}
else
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please, format code like

if () {
} else if () {
} else {
}

@@ -1,17 +0,0 @@
#build time properties
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it was an example of gradle.properties file, I don't think that we should remove it

zipStorePath=wrapper/dists
distributionUrl=https\://services.gradle.org/distributions/gradle-2.13-bin.zip
#Sun Sep 25 16:18:33 PDT 2016
distributionBase=GRADLE_USER_HOME
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}
else
{
cCompiler.args "/DWIN32", "/D_WINDOWS", "/IC:/Program Files (x86)/Microsoft Visual Studio 14.0/VC/INCLUDE", "/IC:/Program Files (x86)/Microsoft Visual Studio 14.0/VC/ATLMFC/INCLUDE", "/IC:/Program Files (x86)/Windows Kits/10/include/10.0.10586.0/ucrt", "/IC:/Program Files (x86)/Windows Kits/NETFXSDK/4.6.1/include/um", "/IC:/Program Files (x86)/Windows Kits/10/include/10.0.10586.0/shared", "/IC:/Program Files (x86)/Windows Kits/10/include/10.0.10586.0/um", "/IC:/Program Files (x86)/Windows Kits/10/include/10.0.10586.0/winrt"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there are many common parts of paths here, it would be nice if it can be moved to reused variables/properties

@@ -9,96 +9,96 @@ jobject getNotificationType(JNIEnv * env, OpenZWave::Notification::NotificationT
const char * name;
switch(ozwNotificationType)
{
case OpenZWave::Notification::Type_ValueAdded:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please, use spaces insead of tabs

@@ -2125,7 +2165,12 @@ JNIEXPORT void JNICALL Java_org_zwave4j_Manager_addWatcher
env->NewGlobalRef(notificationWatcher),
env->NewGlobalRef(context)
);
notificationWatchers[std::pair<jobject, jobject>(notificationWatcher, context)] = pair;

// This key is deallocated after this scope closes causing numerous issues.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm quite poor at c++, but I thought that key is copied into a map as it is a value-type. Can you please give an example of issue?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was so long ago. I believe what was occurring was it was throwing an exception when removing the watcher because the key was garbage collected. By de-referencing the pointer to the actual structure into that table, it is never garbage collected until' "delete" or "free" is explicitly called on the pointer.

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.

None yet

4 participants