Sending commands to devices implementation
I like the idea of templates, but how are you going to deal with binary protocols?
There are a lot of changes and I am a bit scared of merging all of it. I though it would be a good idea to start with single protocol or maybe two protocol (one text and one binary) and see if the approach works and fix any issues that come out.
As for existing libraries, there are some historical reasons for that. Originally I though that I don't even need model classes on the server side and I can convert database result sets directly into JSON and send to client. But after all I ended up with my own implementation of GSON. I think it's simple enough to keep it instead of relying on the third party library. Also, it is easier to customize if I need some special behaviour in future.
What would be an example of a binary protocol? Maybe those cases cannot be resolved using templates, as it is likely that each protocol would need to encode the information differently.
If you can point me to an example of a binary protocol, I can try to modify the design to adopt it and then we can see from there.
The only bad thing for me is that I won't have a physical gps to try it out :(
One very popular example of binary protocol is GT06. You can find documentation here:
https://www.traccar.org/protocols/
I updated the branch with the implementation of two commands from GT06 (the ones matching the commands I defined before).
What I end up realizing is that maybe not all protocols can be easily modeled as a generic to all protocol template. I extended the solution so that it can contemplate both cases.
There is a StringCommandTemplate for commands that can be implemented with Strings, and it can always be used the interface directly (CommandTemplate) to create an object that can implement a non generic command. This is the case for the GT06, which has some "complicated" field dependent calculations. For the GT06 the commands are implemented in a particular subclass, since all commands are the same within the protocol but do not have any meaning with other binary protocols.
What I couldn't test, obviously, is if the specific implementation of the GT06 is correct as I do not have a real GT06 GPS devices. Nonetheless, I think that if that implementation has some bugs, they are local to the particular command and not to the design.
Do you think it is better now? What else could be improved? What else is it plain wrong?
Thanks in advance for the feedback!
I haven't got time to look through all the changes in details yet, but in general I think it looks good.
One problem I noticed so far is that your implementation doesn't support UDP, but it should be fairly easy to solve. You probably need to add remote address to ActiveDevice for that.
Now we need to think how to merge all those changes into the main project. It would be great if you could split it into smaller pull requests that I can review one by one. Do you think it's possible?
You are right, I missed passing the remoteAddress down to the write in the channel. I'll add that tomorrow.
How would it be useful for you to organize the pull request? Do you want me to order like in refactor steps? For instance:
gprs-command-1-JSON-extension (only the part of adding nested objects)
gprs-command-2-ActiveDevice (only the part of asociating the device to the channel)
gprs-command-3-servlet-raw (only the part of sending a raw command)
gprs-command-4-servlet-send-command (only the part of sending commands for one protocol)
gprs-command-5-servlet-send-command-templates (adding the templates)
gprs-command-6-protocol-hierarchy-refactors (moving the protocol specifics to the hierarchy)
So that would be one commit for each step, and one pull request for each commit.
But if you have another idea how to organize it, tell me and I'll do it :)
Sounds good. Waiting for pull requests.
Everything is committed, pushed and pulled requested.
As we agreed I created a branch for each change with a pull request per branch. Now that I look at it, maybe I don't know how easy GitHub will do it for you, since when it showed me the changes for branch 2, it included the changes for branch 1. So maybe if you merge the first pull request, the second one will look cleaner.
Maybe it will be easier for you to clone the last branch, and see the differences between commits. But you can do it either way, whatever it is more comfortable to you.
I hope everything goes ok!
Thanks. I don't think I would have time to review and merge all of it this week because I plan to finish new web interface. So, most likely next week. I will let you know if I have any questions/problems.
I have merged all the changes in one go.
Most of it looks fine, but I want to get rid of enum because I think it makes things more complicated than it should be. I don't see the point of serializing enum as we can just use string type for the same purpose.
Thanks for contributing to the project.
Hi, good to know it was good enough. For next time we know that it is better to arrange for a single pull request, so that it is easier for you. I can squash all the commits next time, so you don't have to do it yourself like this time.
With respects to the enums, they are playing to roles in this implementations, one that is easier to remove and the other might need some considerations:
The easiest role is to type check the field. This way only by looking at the enum you know which types of commands are available. This can be replaced with a set of String constants defined in a single place (and then be sure to always put the commands types there)
The second role is to know which subclass of command (of GpsCommand) instantiate for the given request. Since all commands are polymorphic there has to be a way to create the corresponding subclass, and that information is in the enum (look at the constructor, where it receives the specific class to instantiate for each type).
This is allows to have all the commands typed-checked since there will be one subclass for each command, which can be easily typed.
To change the second role, the different models I can think of are:
a) Not to type each command explicitly. So there will be a single GpsCommand class, with a JsonObject as its data attribute, and then for each template defined how to get each parameter.
b) Some libraries add a field specifically to know which class the json maps to. So the input JSON would be something like:
{
"class": "org.traccar.http.commands.FixPositioningCommand"
"uniqueId": "NNNNNNNNNN",
"command": "FIX_POSITIONING",
"data": {
"unit": "SECOND",
"value": "10"
}
}
but that would make changing the name or package of a class a big change, since it will break the API
c) We could create a big case for each String constant to map against each GpsCommand subclass. Although that would be similar to what it is doing now automatically.
I hope I could explain the design clearer. Maybe you can think another way to redesign this part or some of this other solutions are good enough for the project. Either way, I'll be glad to make the changes necessary for it.
Thanks for all the reviews!
such as sending direct command by / api /? What json format need to send?
I am planning to change API a little bit, so you might want to wait for the new version.
Demian I am trying to contact you seeking for some help to try your fork but I donnot how, seems there is no PM here nor in Github
I am from spain and I really need the commands, because in spain sms are outrageous, thats why this is so important
but I don't know how to install all this.
It's just replace the files with yours ?
if you could write me a message would be great.
You can do it at "nomedeslabrasa" and after that the most known email server in the world, the one of the known search engine ( Trying to avoid some spam here :-) )
Anyway I am using a device that uses H02 protocol and till know I am trying an app called packet sender to listen to the packets coming from the device and replying in order to try to find out the gprs commands and would be great to give a try with your fork.
I don't know almost anything about programming but if I can help anyway to the main branch or yours I can give a try.
Thanks a lot
Hello Anton,
I am not sure where to write about this, since there is an open issue in github (#7), but I will go here since it seems more constructive discussion oriented :)
Since I am in need of sending commands to the gps devices I implemented the feature and so I wanted to share the code with you. First of all, I made quite a bit of code movements, which I'll explain below, so don't panic, everything can be changed.
All the work is in this branch (which is rebased against your master as of right now): https://github.com/demianalonso/traccar/tree/gprs-commands
The biggest change you'll find is that I created a Protocol hierarchy so all protocols can be treated polymorphically for whatever operation is needed. To identify which protocol corresponds to what devices, I hung on the identify method on the ProtocolDecoder class. So as soon a device connects to the server, it gets associated with the protocol it connected. This seemed better than selecting the protocol for each device manually since: a) If you change your SIM to another gps with a different protocol, everything is handled automatically; b) It is tedious to set it manually.
This hierarchy allowed me to put the command templates specifically for each protocol. And I also moved the TrackerServer initialization to this hierarchy, so everything that is related to a given protocol, can be found starting in its corresponding Protocol subclass.
The command message is split in two parts. The first part is the template for each command which is defined in each protocol subclass in loadCommandsTemplates (which is protocol dependent). The base implementation is just a Map of Command Type => String, where the String is the template which has placeholders for the actual values. The second part is the execution of the message (BaseProtocol.sendCommand) where it takes the template and changes the parameters according to the current requested command.
As far as I understand (I am still new to GPRS GPS command protocols) all the parameters for a command type (for instance, to set the frequency of locations notifications) are the same, but the format of the message is different. So for each type of command there is a hierarchy called GpsCommand which defines this common part.
Again, as far as I understand (but this time I am less sure he), the protocols usually sends a single message to the gps and so this implementation covers that case. But if it were that the a protocol has a more complicated definition, then it can override the sendCommand message and implement it that way, without affecting the rest of the code.
This takes me to why did I defined the commands in code, instead of putting them in the database or in the configuration file. Mainly, I believe that this is an implementation issue which is not something the user should modify, as the protocol will not change. Besides, if the protocol needs some more logic, then it is more natural to handle it in code instead of a configuration file/database.
Yet, I did something complementary. Since I am not familiar with this protocols, I created a servlet with a "debug" service that send a raw command (literally whatever the user sent in the json) to the gps. That allowed me to try different commands to see what they do (since, hey, protocol documentation is not something that is really accurate... or available :( ).
It is called debug, but it could also be called ad-hoc, and would give the feature of sending an ad-hoc command without the need of having it inside traccar. So if someone has a GPS with not so common feature or protocol extension, then it can use it right away.
On a less important note, I extended the JSON converter API to handles enums and nested objects (this is necessary for each command subclass). I also created some test for all the converters. It is working now but made me wonder why isn't traccar using any existing libraries (like GSON) to parse and produce JSON. Just a little digression but I am curious about it.
Well I think this covers everything and I really hope that you are interested in adding this feature in your current roadmap. And sorry for the length of the post, but I wanted to explain all the decisions I took so you can evaluate it better. If you have any question about it I am more than willing to answer them.
Any suggestions is, of course, very appreciated. If we should have this discussion in github (whether in the issue or as a pull request), let me know and I'll move all of this hehe
Thanks in advance!