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.

Sign up for GitHub

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

Jump to bottom

feature: Add support for ShipModul Miniplex Latest Firmware #234

Merged
merged 9 commits into from
May 2, 2023
Merged

feature: Add support for ShipModul Miniplex Latest Firmware #234

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
9fbdc02
Added changes to accomodate ShipModul Miniplex
jprotzman Dec 2, 2022
cf63345
Updated test to include R for $PSMDST sentence
jprotzman Dec 2, 2022
8e451e8
Added both original and new ShipModul decoding
jprotzman Dec 2, 2022
5b30229
Updated test to include 'R' field from ShipModul
jprotzman Dec 2, 2022
0dea5a1
Update PSMDST.js
jprotzman Feb 14, 2023
3a8ea04
Update PSMDST.js
jprotzman Feb 14, 2023
c9d8e5f
Update seatalk.js
jprotzman Feb 17, 2023
5656af4
Update seatalk.js
jprotzman Feb 17, 2023
62eb50c
Update seatalk.js
jprotzman Feb 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 12 additions & 8 deletions hooks/proprietary/PSMDST.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,22 +17,26 @@
'use strict'

/*
0 1 2 3
| | | |
$PSMDST,xx,yy,nn*CS
0 1 2 3 4
| | | | |
$PSMDST,Z,xx,yy,nn*CS
where:
PSMDST Raymarine Seatalk1 datagram sentence
0 00-9C Datagram type
1 hex First datagram content
2 hex Last datagram content
3 hex Checksum
0 C/R R for Recevied messages, C for sent messages *Note: This field only exists in later firmware versions of the ShipModul Miniplex
tkurki marked this conversation as resolved.
Show resolved Hide resolved
1 00-9C Datagram type
2 hex First datagram content
3 hex Last datagram content
4 hex Checksum
*/

const seatalkHooks = require('../seatalk')

module.exports = function (input, session) {
const { id, sentence, parts, tags } = input
const key = '0x' + parts[0].toUpperCase()
if (parts[0].toUpperCase() === 'R') {
tkurki marked this conversation as resolved.
Show resolved Hide resolved
input.parts = parts.slice(1, input.parts.length);
}
const key = '0x' + input.parts[0].toUpperCase();
if (typeof seatalkHooks[key] === 'function') {
return seatalkHooks[key](input, session)
} else {
Expand Down
2 changes: 1 addition & 1 deletion test/seatalk.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ const should = chai.Should()
chai.use(require('chai-things'))

describe('seatalk', () => {
;['$PSMDST,', '$STALK,'].forEach((prefix) => {
;['$PSMDST', '$PSMDST_R', '$STALK,'].forEach((prefix) => {
Copy link
Member

Choose a reason for hiding this comment

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

If I understood this PR correctly this test is not really relevant: it now tests the normal input format, but for PSMDST_R. Shouldn't we actually add a test where input does indeed have the extra R in the data?

Does the parser ever see the C data?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought this test was testing all of the typical input formats. It previously had both $PSMDST and $STALK which are typical from various devices. The new Miniplex firmware adds the _R for all received data so this test just adds that option as well.

Also, the _C field may appear at the parser but I cannot imagine these would ever need to be parsed since they are sent messages meant to go onto the ST network.

Copy link
Member

Choose a reason for hiding this comment

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

Several different things:

  • You added support to a new data format, where the first (optional) data field has valueR. That looks like it works, but none of the tests exercise it - you are running the tests that were already in place, with exact the same data fields as previously, no R field, just for an additional prefix
  • the tests do not pass for PSMDST prefix because you deleted the trailing comma in the prefix value (see the diff above, it is highlighted)
  • the tests do not pass for PSMDST_R prefix because there is no trailing comma in the new prefix value

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be corrected now.

it(`${prefix} 0x00 depth converted`, () => {
const fullSentence = utils.appendChecksum(`${prefix}${depthData}`)
const delta = new Parser().parse(fullSentence)
Expand Down