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

Provide possibility to set "exotic" ports' names #179

Open
andrey-patin-1979 opened this issue Dec 22, 2024 · 15 comments
Open

Provide possibility to set "exotic" ports' names #179

andrey-patin-1979 opened this issue Dec 22, 2024 · 15 comments
Labels
enhancement New feature or request good first issue Good for newcomers language:java Needs changes to java sources.

Comments

@andrey-patin-1979
Copy link

Some drivers for different OSes can call serial ports with different "exotic" names (e.g. "muiO" for Moxa on Linux (https://moxa.com/en/support/product-support/software-and-documentation?psid=114475)).
It would be helpful if there were possibility to provide such "exotic" ports names into the library.

@andrey-patin-1979
Copy link
Author

I hope can provide solution for this issue on next week.

@tresf
Copy link

tresf commented Dec 22, 2024

The page linked does not reference a specific port pattern, but has documents that allude to the port pattern.

  • The pattern for Windows seems to leverage the existing COM# pattern and no changes should be needed
  • The pattern for macOS seems to leverage the existing /dev/tty.USB* pattern and no changes should be needed
  • The pattern for Linux seems to leverage /dev/ttyMXUSB* pattern and will require a patch.

Relevant code:

case SerialNativeInterface.OS_LINUX: {
PORTNAMES_REGEXP = Pattern.compile("(ttyS|ttyUSB|ttyACM|ttyAMA|rfcomm|ttyO)[0-9]{1,3}");
PORTNAMES_PATH = "/dev/";
break;
}

Excerpt from MOXA manual:
Screenshot_20241222-114640~2

@tresf
Copy link

tresf commented Dec 22, 2024

"exotic" names (e.g. "muiO" for Moxa on Linux

@andrey-patin-1979 Please link direct documentation to support this statement.

@tresf tresf added enhancement New feature or request good first issue Good for newcomers language:java Needs changes to java sources. labels Dec 22, 2024
@andrey-patin-1979
Copy link
Author

andrey-patin-1979 commented Dec 23, 2024

tresf, for instance in the https://www.moxa.com/getmedia/bf07835c-fdca-43c2-8879-0eaa24bdcd4d/moxa-industrial-linux-3-debian-11-manual-with-security-hardening-guide-v1.0.pdf / section "Serial Port" (on page 31) as an examples of names stated following "Device node (e.g., /dev/ttyM0)", and another example "exotic" port naming for MOXA is here "https://docs.moxa.online/mil/manuals/tools/mx-uart-ctl/#da-680".
Actually I faced this "names problem" for Moxa on Linux when using this one driver (https://github.com/uecasm/mxser/blob/3188030d088e2f12bec4de91956be16488e3bcd5/driver/msmknod#L13), but I'm not sure that it is only example since a lot of different devices are developed so far and can be present in different OSes with such "exotic" names.

@tresf
Copy link

tresf commented Dec 23, 2024

I'm not sure that it is only example since a lot of different devices are developed so far and can be present in different OSes with such "exotic" names.

Right, but we can only code for what we know.

So far the proposal as we've identified are:

 case SerialNativeInterface.OS_LINUX: { 
-    PORTNAMES_REGEXP = Pattern.compile("(ttyS|ttyUSB|ttyACM|ttyAMA|rfcomm|ttyO)[0-9]{1,3}"); 
+    PORTNAMES_REGEXP = Pattern.compile("(ttyS|ttyUSB|ttyACM|ttyAMA|rfcomm|ttyO|ttyM|ttyMXUSB|ttyMUE)[0-9]{1,3}"); 
     PORTNAMES_PATH = "/dev/"; 
     break; 
 } 

This adds three new options that are compatible with Moxa's documentation:

  • /dev/ttyM<number>
  • /dev/ttyMXUSB<number>
  • /dev/ttyMUE<number>

We may choose to recognize all /dev/ttyM* patterns as valid serial ports, however the regex would need to change for that to work, so adding the three separate patterns is what I would recommend for now.

@andrey-patin-1979
Copy link
Author

@tresf, actually I was thinking about adding special Java-option that will allow to add "exotic names" to PORTNAMES_REGEXP for any OSes. It looks more convenient way to fix this issue once and for all. Do you agree?

@tresf
Copy link

tresf commented Dec 24, 2024

@tresf, actually I was thinking about adding special Java-option that will allow to add "exotic names" to PORTNAMES_REGEXP for any OSes. It looks more convenient way to fix this issue once and for all. Do you agree?

I don't see this feature being particularly useful for Windows, but I think it would be a nice addition to the API regardless due to its usefulness on macOS and Linux. Like any feature, it may be a security concern for companies to offer an arbitrary codepath to flat file reads, so I think this feature should have an option to disable it. I still believe that the library should "Just work", so adding values -- such as those introduced by MOXA devices -- is still desired, they've been in the industry for decades.

@andrey-patin-1979
Copy link
Author

Just to make sure I understand you, @tresf, correctly, you want to add this feature by yourself?
Or you want me to make MR with this feature, in that case what kind of change you want to see in the MR from me, only update of PORTNAMES_REGEXP or add java-option (if you mean this by API) or both?

@tresf
Copy link

tresf commented Dec 25, 2024

I would ask for two PRs, one with well-known names one with the REGEX/future unknown names feature.

@andrey-patin-1979
Copy link
Author

I would ask for two PRs, one with well-known names one with the REGEX/future unknown names feature.

OK, would you, @tresf, please create a branches for both of these changes (seems I don't have rights to do it)?

@tresf
Copy link

tresf commented Dec 25, 2024

You would normally create your own fork, then your own branch, then PR.

@tresf tresf changed the title Provide possibility to set "exotic" ports' names Detect MOXA ports on Linux Dec 28, 2024
@tresf tresf changed the title Detect MOXA ports on Linux Provide possibility to set "exotic" ports' names Dec 28, 2024
@andrey-patin-1979
Copy link
Author

@tresf, 1-st PR created (#180)
I can't run tests (I developing on Windows and "mvn clean test" can't run tests successfully even when I placed jssc.dll from "src\main\resources-precompiled\natives\windows_64" to the root of my working drive), they are seems failed since can't receive expected exception "UnsatisfiedLinkError" (java.lang.AssertionError: Library loading should fail if path provided exists but does not contain a native library
at jssc/jssc.bootpath.ManualBootLibraryPathFailedTest.testBootPathOverride(ManualBootLibraryPathFailedTest.java:27))

I don't know if I need to create any tests for this PR (with changed value of PORTNAMES_REGEXP) or in 2-nd PR for new option, if I need to create test(s) then please provide me an information how to run them.

@tresf
Copy link

tresf commented Dec 28, 2024

I don't know if I need to create any tests for this PR

I appreciate the offer. At time of writing this, I don't think we would need tests for all the different port patterns -- at least not the scope of #180 -- although I'm not opposed to having Linux tests that simulate the various port names, I wouldn't expect you to spearhead this effort for such a simple change. Perhaps we add this requirement before merging the next PR with a dynamic REGEX pattern, but this next PR may have other requirements (such as any raised security concerns) that will naturally prolong its merge.

tresf pushed a commit that referenced this issue Dec 28, 2024
Allows matching of MOXA device port names in Linux
Related: #179
@andrey-patin-1979
Copy link
Author

@tresf, in that case I need to know how to run tests at least on Windows, could you provide me such information (since I didn't find anything in Wiki and mvn clean test didn't make a trick too.

@tresf
Copy link

tresf commented Jan 1, 2025

I need to know how to run tests at least on Windows

Why? This request/feature is for OSs that allow arbitrary POSIX paths to be used for Serial communication. Windows doesn't allow this.

With regards to why tests aren't running... JUnit skips all Windows tests in the VirtualPortRule file -- which I find to be tremendously confusing -- but I don't write a lot of unit tests, so take my opinion with a grain of salt.

public Statement apply(final Statement base, final Description description) {
// skip / prevent deadlock if socat isn't available
if (SerialNativeInterface.getOsType() == SerialNativeInterface.OS_WINDOWS || !execute("socat", "-V")) {
return new Statement() {
@Override
public void evaluate() throws Throwable {
base.evaluate();
}
};
}

With regards to creating unit tests for Linux, this would likely require adding a symbolic link to the filesystem that matches a the arbitrary pattern proposed in this #179 (comment).

e.g.

export _JAVA_OPTIONS="-Dportnames.match="(ttyFOO|ttyBAR)[0-9]{1,3}"

With regards to how to make the test ALSO use that port pattern, I'm at a bit of a loss. I did not write these unit tests, but I think you can find them somewhere around here:

LOG.info("Process started! [{}], ports: [{}] // [{}].", this.process, VirtualPortRule.this.virtualCom1.getName(),

What I'm not sure about is whether or not we actually use the pattern matching in our tests. If not, that's something we should probably fix. We would expect the enhancement PR to help with this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers language:java Needs changes to java sources.
Projects
None yet
Development

No branches or pull requests

2 participants