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.

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

Fix for NMEA loop to resend to it self. #1476

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nauticat
Copy link
Contributor

I have recently installed a NMEA hardware HUB in my boat. After some debugging I realized that O was sending back received sentences from the hardware HUB - starting an infinite loop. However, due to limited buffer sizes the NMEA bus survives but I really have problems with sentences that disappears.

I did not have time to figure out the full potential of the Multiplexer in O but from my point of view this is not a good behavior. Suggest a correction to fix this problem and also enable Multiplexing to networks.

/fredrik

@transmitterdan
Copy link
Collaborator

In the options->connections dialog you can tell a connection not to send any data by telling it to drop all NMEA sentences at the transmit setting. Did you try that before changing the code?

@nauticat
Copy link
Contributor Author

Definitely, your suggestion is a workaround, I tested that first and that will solve my problem. However, it is complicated (not user friendly) and maybe not the intention of the Multiplexer in O.

As far as I understand, the intention of the Multiplexer is to cross transfer information between different ports but never to resend received information to itself (I cannot find out any reason to do that). I guess it was just a simple mistake in the code an OR instead of an AND.

@transmitterdan
Copy link
Collaborator

I am pretty sure it is not a mistake in the code. Most real devices ignore messages they get from themselves. They look at the two letter source and when they see a message from themselves they ignore it.

OpenCPN is often used to bring multiple sources together and then transmit them out over the network. I worry that your change will break many users's setups.

@nauticat
Copy link
Contributor Author

I totally agree with you, I do not want to contribute to a solution in O that break existing installations ... I just want to contribute to a good software and share my experience on how to handle a slightly bigger installation. Maybe someone else can test or confirm this?

I have five active ports defined in my O setting now, 4 NMEA via USB and 1 UDP broadcast network. Two of the USBs are connected to NMEA HUBs which in turn connects many devices. I am really depend on O bringing all information together and transmit it back to the network (multiplexing). However, in such an installation, the problem is easy to see i.e. an overloaded NMEA buss due to low bandwidth (and maybe too many devices :-) But many devices is not so uncommon these days.
The effect is not notable in a smaller installation with a few units.

To help me understand please tell me what is the purpose of O echoing the same information received from one device back to the same device again when we have a limited bandwidth? At this point I do not get it and why this is not a mistake? Sorry that I lack some knowledge here, I have not worked with the multiplexer code in O before.

@RooieDirk
Copy link
Contributor

My advise to users is: Don't output on a port unless you really need to.
If you really need it, make sure you filter the output to only the needed sentences, and then also make sure to block the input for that port to the same as the output sentences.

To check every sentence for 'being send before' would be a large overhead I'm afraid, but do a check and a warning message while settingup a connection could be easy done.

@nauticat
Copy link
Contributor Author

nauticat commented Dec 3, 2019

RooieDirk great advice I agree on how to set up NMEA in general.

However, regarding the “duplication of messages back to self” bug. I have now tested the fix for a while. In my installation it works great now, for many hours of sailing, as opposed to before.

@rgleason
Copy link
Collaborator

rgleason commented Dec 3, 2019

I see this
9d126a4

A hidden action assuming the user never has a need to send that data out again, and no control over it? Is that appropriate?

@nauticat
Copy link
Contributor Author

nauticat commented Dec 3, 2019

No problem, the user can send any data out again as expected by the MUX. However, the MUX should never echoing exactly the same received message from a device back to the same device again. This will create an unwanted/unexpected duplication of the message and a high risk of jeopardizing the bus.

It is also stated in the comment in the code that it should only send to all other. As one should expect from a MUX.
//Send to all the other outputs

I guess it was only a mistake in the logical statement in the if expression.

@rgleason
Copy link
Collaborator

rgleason commented Dec 3, 2019

No problem, the user can send any data out again as expected by the MUX. However, the MUX should never echo exactly the same received message from a device back to the same device again.

Basically, I agree with you, does anyone else?

@pgrawehr
Copy link
Contributor

I'm not sure I understand the full context here, but I can imagine a setup that would break there: On one UART port, I have a GPS device on the RX line (since that only sends data) and the Autopilot on the TX line (since that one only has an input). So I would eventually want (at least some messages) to be directly forwarded to the same port.

@transmitterdan
Copy link
Collaborator

I'm not sure I understand the full context here, but I can imagine a setup that would break there: On one UART port, I have a GPS device on the RX line (since that only sends data) and the Autopilot on the TX line (since that one only has an input). So I would eventually want (at least some messages) to be directly forwarded to the same port.

I agree. There is no guarantee that a serial port input and output are connected to the same device. Many times they are not. So the code should not assume that a serial port output is connected to the same device as the input. So far as I know only one user has identified a problem. There should be ways using output filters to solve the problem. I am interested in learning why an output filter is not the solution.

@nauticat
Copy link
Contributor Author

I agree, I can see that it will break such an installation and that is not good. (On the other hand it will make it easier to handle bidirectional communication.)

However, I can’t get the output filter to work for an NMEA hub/device that reuse the same sentence for several different purposes. E.g. I have a NME0183 to SEATALK device that always send information as $TALK, xxxx and I need to send back new information as $STALK, yyyy. But a filter will delete both or?

@rgleason
Copy link
Collaborator

Filters in and filters out are used.

@transmitterdan
Copy link
Collaborator

Rather that speak in hypothetical let's discuss a real case. What sentence exactly do you receive from the Seatalk device? And what sentence exactly do you want to send back to the Seatalk device? If the sentence you want OpenCPN to send requires some kind of editing (like sed might do) then OpenCPN does not have such capability. I think OpenPlotter may have some utilities that can do something like that. Creating a SignalK server would possibly do what you want. OpenCPN will be able to receive data from SignalK server in next release.

@rgleason
Copy link
Collaborator

There are bidirectional seatalk -nmea0183 devices. These devices can often be configured to filter or not send or receive certain sentences.

Trying to do a similar thing using the bidirectional converter but using Opencpn in and out filters is probably not as effective as configuring and programming the device itself.

@nauticat
Copy link
Contributor Author

How about a new check box in the configuration dialog. Bidirectional communication on/off (or something better description of it?). Then O can handle both situations.

I agree that it is possible to do it in another way. But it is not that user friendly. Many new devices are bidirectional e.g. a new AIS transponder is usually equipped with an USB port.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants