Skip to content

Bugfix: Add VGN mode parameter #5523

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

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

Conversation

incognito53
Copy link

GV50M tracker was flipping Online - in a minute due to NumberFormatException in decodeIgn's parsing of position data.
The root cause was a missing index += 1 to skip over the virtual ignition mode parameter.
image

@tananaev
Copy link
Member

  1. We need to add a test case
  2. Are you sure it's applicable to all devices?

@tananaev
Copy link
Member

Looks like existing tests are failing.

@incognito53
Copy link
Author

Ehh realized that when it broke IGN for some devices. Updated.

Map.entry("BD", "CV200"),
Map.entry("C2", "GV600M"),
Map.entry("DC", "GV600MG"),
Map.entry("DE", "GL500M"),
Map.entry("F1", "GV350M"),
Map.entry("F8", "GV800W"),
Map.entry("FC", "GV600W"),
Map.entry("FE", "GV53MG"),
Copy link
Member

Choose a reason for hiding this comment

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

Please attach full documentation for both device models.

@@ -1074,6 +1077,11 @@ private Object decodeIgn(
return position;
}

private boolean isVirtualIgnitionMode(String type, String model) {
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need a separate function for this. Also what does type.contains("VG") mean?

@@ -1074,6 +1077,11 @@ private Object decodeIgn(
return position;
}

private boolean isVirtualIgnitionMode(String type, String model) {
return type.contains("VG") &&
List.of("GV50M", "GV53MG").contains(model);
Copy link
Member

Choose a reason for hiding this comment

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

A set of direct comparison would make more sense here.

@@ -61,6 +61,9 @@ public void testDecode() throws Exception {
"+RESP:GTVGN,C2010D,869653060009939,GV600M,453966,0,0.0,0,163.9,10.239379,45.931389,20231210233723,0222,0010,2F31,006D7621,,,0.0,20240107182623,143F$"),
Position.KEY_IGNITION, true);

verifyPosition(decoder, buffer(
Copy link
Member

Choose a reason for hiding this comment

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

Please add new test to the top of the file.

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.

2 participants