Add basic support for VSI sentences#30
Conversation
This takes advantage of the fix introduced in commit e4b88c0.
|
Hi @johnmartel - thanks a lot for this contribution. It requires to be reviewed on our side before entering the master branch - this will currently be pending due to time constraints; but as soon as I get sufficient time, I shall take a look. One immediate comment is that I would like to avoid the .mvn/ folder to be included - but I may need to understand better why it is there. Thanks again for this contribution. |
|
Hi Thomas,
the .mvn folder is required by the Maven wrapper. This is actually quite useful, as it enables anyone that clones this repo to build it without requiring a maven install or one asking the question which version of Maven works with this project.
You can have a look at the Maven wrapper or the Gradle wrapper, as the idea comes from Gradle and they have a pretty good documentation about it.
… On Mar 29, 2017, at 15:02, Thomas Borg Salling ***@***.***> wrote:
Hi @johnmartel <https://github.com/johnmartel> - thanks a lot for this contribution. It requires to be reviewed on our side before entering the master branch - this will currently be pending due to time constraints; but as soon as I get sufficient time, I shall take a look.
One immediate comment is that I would like to avoid the .mvn/ folder to be included - but I may need to understand better why it is there.
Thanks again for this contribution.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#30 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AC7SX2CjRgUXZ4K01bYnKOqVQVzcyxHCks5rqqrigaJpZM4MpejI>.
|
|
@tbsalling when reviewing this, if you have any recommendations on writing the code differently to better fit your architecture, please go ahead and I shall make the changes. I am in no way an AIS expert and simply wanted to get things working for the CCG so that they can start analyzing coverage using signal strength. This is actually working quite fine, but I don't like that big "if" statement in the AisPacketParser... |
|
The Gradle wrapper documentation I was referring to: https://docs.gradle.org/current/userguide/gradle_wrapper.html You'll see here that the |
When some stations as misconfigured and do not send the signal strength correctly, this results in a `NumberFormatException` which clogs the logs and makes it difficult to read them. Due to the raised exception, the message is also ignored by the system. By defaulting to a ridiculously low value, the message is accounted for by the system, while making it clear that something is wrong with the station.
Using `Integer.MIN_VALUE` is not a good idea because signal strengths are summed to compute an average. Summing up two messages with a signal strength of `Integer.MIN_VALUE` will incorrectly give a positive int.
`AisPositionMessage` already extends `AisMessage` and implements `IVesselPositionMessage`. This simplifies the hierarchy and remove some duplication.
This reverts commit 894fd06.
This allows building the project with Gradle instead of maven. This is particularly interesting because the AisLib project is not pushed to a proper maven repository, which makes it impractical to use the HEAD in other projects. By having a Gradle build, it is easy to use the AisLib git repository as a submodule and depend on it in a Gradle composite build.
|
Hi @johnmartel thanks again. I plain simply haven't had time to review this yet; but your contrib is appreciated and not forgotten! |
|
Hi @johnmartel - I know that this PR has been dangling for quite a while. The reason for this long idle period is that we are not set to embody VSI sentences in the general product. An additional minor reason is that we do not intend to support both Maven and Gradle. So far we are (still) going with just Maven - and not (yet) the wrapper version. So I will mark this PR wontfix for now (to reflect the de facto state, which you may already have deduced :-)). We do still very much appreciate your contribution and use of AisLib, though - so thanks for contributing; we hope that you are still happy and productive with the tool. |
2 participants
This comes as a request from the Canadian Coast Guard who are using the dma-ais/AisCoverage project and wanted to be able to analyze coverage by signal strength.
I had a limited amount of time to work out a solution, so this may not be as clean as I would like it to be.
The idea is pretty simple:
\c:[timestamp to the second],C:[message counter for this station],s:[station unique id]*[checksum]\This PR goes hand in hand with this pull request on dma-ais/AisCoverage.
When merging this PR, one should be aware that the version number in pom.xml now assumes that semantic versioning is used and that the CCG pre-release identifier has been added to identify my current builds as being from the fork. I think you should get rid of the pre-release identifier, as this will be merged in the original, official repo. A new official distribution with these changes would be greatly appreciated.
Please reach out with any comments/questions :)
Thanks!