-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Wialon: interpret LBS data from parameters #5502
base: master
Are you sure you want to change the base?
Conversation
@@ -154,4 +154,7 @@ public long getLong(String key) { | |||
} | |||
} | |||
|
|||
public void removeAttribute(String key) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're adding a new method like this, we should update all the existing places where we could use it currently. We should do a separate PR for that.
@@ -156,11 +154,54 @@ private Position decodePosition(Channel channel, SocketAddress remoteAddress, St | |||
} | |||
} | |||
} | |||
|
|||
// interpret accuracy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's remove all the obvious comments like this
if (position.hasAttribute("mcc")) { | ||
interpretLBSAndRemoveAttribute(position, network, ""); | ||
} | ||
if (position.hasAttribute("mcc1")) { | ||
for (int i = 1; i <= 9; i++) { | ||
interpretLBSAndRemoveAttribute(position, network, String.valueOf(i)); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can probably do a single loop for all of these. But would be nice to break early instead of going through everything.
} | ||
|
||
return position; | ||
} | ||
|
||
private static void interpretLBSAndRemoveAttribute(Position position, Network network, String suffix) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would probably rename this to something like decodeCellData
or something like that.
import org.traccar.model.Command; | ||
import org.traccar.model.Position; | ||
import org.traccar.model.WifiAccessPoint; | ||
import org.traccar.model.*; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't do star imports. And also let's separate all the test refactoring in a separate PR as well.
CellTower.from(250, 2, 5901, 45542), | ||
CellTower.from(222, 22, 2222, 22222) | ||
)) | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Closing brackets should be on the previous line. That's the style we use.
@@ -28,10 +30,10 @@ public void testDecode() throws Exception { | |||
|
|||
verifyNull(decoder, text( | |||
"#L#123456789012345;test")); | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't forget to undo all these changes and clean up your PR. Let me know once it's ready for another review.
According to the Wialon protocol specifications, the LBS data is transmitted as parameters.
https://wialon.com/hw/files/Wialon%20IPS%20v.2.2%20%28RU%29.pdf
Modified the decoder to account for that
This is a draft to gather initial feedback, as we haven't discussed the contribution beforehand. I am still testing changes locally.