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

Define constants manually from preprocessed header files #49

Open
aploese opened this issue Sep 20, 2018 · 7 comments
Open

Define constants manually from preprocessed header files #49

aploese opened this issue Sep 20, 2018 · 7 comments

Comments

@aploese
Copy link

aploese commented Sep 20, 2018

Why not code the constants manually by using preprocessed files like this for termios.h on Linux:
echo "#include <termios.h>" > c.c
gcc -dD -dI -E c.c > _termios-prepocessed.h
Doing this once will give you all defines,structures, function calls in _termios-prepocessed.h, so you can later check or add something even if you have no access to the said hardware?

I have no idea how the copyright is affected if the preprocessed files are stored in the source repository.

And maybe go away from enums to abstract classes that hold the defined values in final public (long|int|short|String) fields?

@headius
Copy link
Member

headius commented Sep 21, 2018

On the public static final fields: probably fine if we assume platforms are not changing individual values. May actually be less fragile than enums. However enums are currently used because they give type safety; you know that you're passing an Errno or a SocketOption until a native call eventually unwraps it for you (or you unwrap it yourself for whatever reason). We lose that type-safety if we go with simple fields.

It's a about equal, maybe.

The library is currently designed around being enums, so that probably won't change in the near term.

@aploese
Copy link
Author

aploese commented Sep 21, 2018

They are noit static Termios_H.java

`public abstract class Termios_H<T extends Termios_H.Termios> implements JnrHeader {
@nullable
final public Defined _HAVE_STRUCT_TERMIOS_C_ISPEED = _HAVE_STRUCT_TERMIOS_C_ISPEED();

@Nullable
final public Defined _HAVE_STRUCT_TERMIOS_C_OSPEED = _HAVE_STRUCT_TERMIOS_C_OSPEED();

/**
 * Hang up.
 */
@POSIX
final public int B0 = B0();

protected abstract Defined _HAVE_STRUCT_TERMIOS_C_ISPEED();

protected abstract Defined _HAVE_STRUCT_TERMIOS_C_OSPEED();

protected abstract int B0();

}
and a generic Termios_Lib.javapackage de.ibapl.jnrheader.linux;

import static de.ibapl.jnrheader.Defined.isDefined;

import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

import org.eclipse.jdt.annotation.Nullable;

import jnr.ffi.types.int32_t;
import de.ibapl.jnrheader.Defined;
import de.ibapl.jnrheader.NativeDataType;
import de.ibapl.jnrheader.posix.Termios_H;
import de.ibapl.jnrheader.posix.Termios_H.cfmakeraw;
import de.ibapl.jnrheader.posix.Termios_H.cfsetspeed;
import jnr.ffi.LibraryLoader;
import jnr.ffi.Runtime;
import jnr.ffi.Struct;
import jnr.ffi.TypeAlias;
import jnr.ffi.annotations.TypeDefinition;

public abstract class Termios_Lib extends Termios_H<Termios_Lib.LinuxTermios> implements cfsetspeed<Termios_Lib.LinuxTermios>, cfmakeraw<Termios_Lib.LinuxTermios> {

public static final int B0 = 0000000;

@Override
protected int B0() {
	return Termios_Lib.B0;
}

}
`

and a Termiso_Impl.java:

` ackage de.ibapl.jnrheader.linux.x86_64.gnu;

import de.ibapl.jnrheader.Defined;
import de.ibapl.jnrheader.linux.Termios_Lib;

public class Termios_Impl extends Termios_Lib {
public static final Defined _HAVE_STRUCT_TERMIOS_C_ISPEED = Defined.DEFINED;
public static final Defined _HAVE_STRUCT_TERMIOS_C_OSPEED = Defined.DEFINED;

@Override
protected Defined _HAVE_STRUCT_TERMIOS_C_ISPEED() {
	return Termios_Impl._HAVE_STRUCT_TERMIOS_C_ISPEED;
}

@Override
protected Defined _HAVE_STRUCT_TERMIOS_C_OSPEED() {
	return Termios_Impl._HAVE_STRUCT_TERMIOS_C_OSPEED;
}

}
`
This will overwrite the defines to the actual values.

@aploese
Copy link
Author

aploese commented Sep 21, 2018

@enums
I encounter different enums for different things in Termios and bitsets as well. So in the long run I dont know if it would be better to go in the c-direction - we are here at the border between C and java, arent We?

@headius
Copy link
Member

headius commented Sep 25, 2018

@aploese I think it would be reasonable for us to cleanup/refactor the current generation logic and see what we have at that point. I think it's a little overcomplicated for what it's doing.

We also need to consider the possibility of hardware platform being pulled into this, which will require changes to the generation logic as well.

@aploese
Copy link
Author

aploese commented Sep 25, 2018

This looks overcomplicated, yes. But if you look at the code from the user of JNR side, its a bit different.
Here is the JNI binding: GenericTermiosSerialPortSocket.c.
And here is the work in progress JNR binding: PosixSerialPortSocket.java.
I think the resulting code contains much less boiler plate code then the enum approach I did before.
And the values are not always casted from long to for instance short.

@headius
Copy link
Member

headius commented Sep 26, 2018

@aploese The approach here is much simpler, but it requires us to ship binaries for each platform, correct? The goal of jnr-constants has always been to provide the generated constants without any native dependency, so they could be used with any foreign function interface.

Or perhaps you just mean using this mechanism to generate the enums in jnr-constants on each platform?

@aploese
Copy link
Author

aploese commented Oct 24, 2018

I cant see that the current jnr approach is simpler - all enums are fetched as long and then truncated to maybe short....
On the other hand some flags are used in bit fields as set whith complete different meanings. I think we are at the border to c-code - so it can get sometimes dirty and the end user should not see any native constants anyhow.

So how to implement the termios struct which has platform dependant fields (or is this a new issue in jnr-posix ?)

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

No branches or pull requests

2 participants