Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 6be3c39e authored by Oleg Nitz's avatar Oleg Nitz
Browse files

Address Android API Council feedback for Serial API.

In SerialManager.registerSerialPortListener:
 - Make the listener the last parameter in SerialManager.registerSerialPortListener for consistency and a more idiomatic usage in kotlin clients.
 - Add the throw tag for the IllegalStateException and the @CallbackExecutor annotation.

In SerialPort.requestOpen:
 - Introduce public constants for the flag and define an @IntDef for them.

Bug: 427450020
Bug: 427647872
Flag: android.hardware.serial.flags.enable_serial_api
Test: manual
Change-Id: I266fb182f02eefce5e05a78114f266fbe112dab5
parent d9d83fd9
Loading
Loading
Loading
Loading
+7 −1
Original line number Diff line number Diff line
@@ -21323,7 +21323,7 @@ package android.hardware.serial {
  @FlaggedApi("android.hardware.serial.flags.enable_serial_api") public final class SerialManager {
    method @NonNull public java.util.List<android.hardware.serial.SerialPort> getSerialPorts();
    method public void registerSerialPortListener(@NonNull android.hardware.serial.SerialPortListener, @NonNull java.util.concurrent.Executor);
    method public void registerSerialPortListener(@NonNull java.util.concurrent.Executor, @NonNull android.hardware.serial.SerialPortListener);
    method public void unregisterSerialPortListener(@NonNull android.hardware.serial.SerialPortListener);
  }
@@ -21333,6 +21333,12 @@ package android.hardware.serial {
    method public int getVendorId();
    method public void requestOpen(int, boolean, @NonNull java.util.concurrent.Executor, @NonNull android.os.OutcomeReceiver<android.hardware.serial.SerialPortResponse,java.lang.Exception>);
    field public static final int INVALID_ID = -1; // 0xffffffff
    field public static final int OPEN_FLAG_DATA_SYNC = 4096; // 0x1000
    field public static final int OPEN_FLAG_NONBLOCK = 2048; // 0x800
    field public static final int OPEN_FLAG_READ_ONLY = 0; // 0x0
    field public static final int OPEN_FLAG_READ_WRITE = 2; // 0x2
    field public static final int OPEN_FLAG_SYNC = 1048576; // 0x100000
    field public static final int OPEN_FLAG_WRITE_ONLY = 1; // 0x1
  }
  @FlaggedApi("android.hardware.serial.flags.enable_serial_api") public interface SerialPortListener {
+9 −1
Original line number Diff line number Diff line
@@ -31,6 +31,14 @@ interface ISerialManager {
    /** Unregisters a listener to monitor serial port connections and disconnections. */
    void unregisterSerialPortListener(in ISerialPortListener listener);

    /** Requests opening a file descriptor for the serial port. */
    /**
     * Requests opening a file descriptor for the serial port.
     *
     * @param flags     open flags {@code SerialPort.OPEN_FLAG_*} that define read/write mode and
     *                  other options.
     * @param exclusive whether the app needs exclusive access with TIOCEXCL(2const)
     * @param callback  the receiver of the operation result.
     * @throws IllegalArgumentException if the set of flags is not correct.
     */
    void requestOpen(in String portName, in int flags, in boolean exclusive, in ISerialPortResponseCallback callback);
}
+5 −2
Original line number Diff line number Diff line
@@ -16,6 +16,7 @@

package android.hardware.serial;

import android.annotation.CallbackExecutor;
import android.annotation.FlaggedApi;
import android.annotation.NonNull;
import android.annotation.SystemService;
@@ -77,9 +78,11 @@ public final class SerialManager {

    /**
     * Register a listener to monitor serial port connections and disconnections.
     *
     * @throws IllegalStateException if this listener has already been registered.
     */
    public void registerSerialPortListener(@NonNull SerialPortListener listener,
            @NonNull Executor executor) {
    public void registerSerialPortListener(@NonNull @CallbackExecutor Executor executor,
            @NonNull SerialPortListener listener) {
        synchronized (mLock) {
            if (mServiceListener == null) {
                mServiceListener = new SerialPortServiceListener();
+59 −13
Original line number Diff line number Diff line
@@ -16,17 +16,22 @@

package android.hardware.serial;

import static android.system.OsConstants.ENOENT;

import static java.lang.annotation.RetentionPolicy.SOURCE;

import android.annotation.FlaggedApi;
import android.annotation.IntDef;
import android.annotation.NonNull;
import android.os.OutcomeReceiver;
import android.os.ParcelFileDescriptor;
import android.os.RemoteException;
import android.system.ErrnoException;
import android.system.OsConstants;

import com.android.internal.annotations.VisibleForTesting;

import java.io.IOException;
import java.lang.annotation.Retention;
import java.util.Objects;
import java.util.concurrent.Executor;

@@ -41,6 +46,52 @@ public final class SerialPort {
     */
    public static final int INVALID_ID = -1;

    /** @hide */
    @Retention(SOURCE)
    @IntDef(flag = true, prefix = {"OPEN_FLAG_"}, value = {OPEN_FLAG_READ_ONLY,
            OPEN_FLAG_WRITE_ONLY, OPEN_FLAG_READ_WRITE, OPEN_FLAG_NONBLOCK, OPEN_FLAG_DATA_SYNC,
            OPEN_FLAG_SYNC})
    public @interface OpenFlags {}

    // Note: for FLAG_* constants we use the current "typical" values of the corresponding
    // OsConstants.O_*: they depend on the Linux distro and can differ for exotic distros.
    // We decided not to use an independent set of constants, because it might overlap with the O_*
    // constants, and a user might by mistake use O_* constants with unpredictable results.

    /**
     * For use with {@link #requestOpen}: open for reading only.
     */
    public static final int OPEN_FLAG_READ_ONLY = 0;

    /**
     * For use with {@link #requestOpen}: open for writing only.
     */
    public static final int OPEN_FLAG_WRITE_ONLY = 1;

    /**
     * For use with {@link #requestOpen}: open for reading and writing.
     */
    public static final int OPEN_FLAG_READ_WRITE = 1 << 1;

    /**
     * For use with {@link #requestOpen}: when possible, the file is opened in nonblocking mode.
     */
    public static final int OPEN_FLAG_NONBLOCK = 1 << 11;

    /**
     * For use with {@link #requestOpen}: write operations on the file will complete according to
     * the requirements of synchronized I/O data integrity completion (while file metadata may not
     * be synchronized).
     */
    public static final int OPEN_FLAG_DATA_SYNC = 1 << 12;

    /**
     * For use with {@link #requestOpen}: write operations on the file will complete according to
     * the requirements of synchronized I/O file integrity completion (by contrast with the
     * synchronized I/O data integrity completion provided by FLAG_DATA_SYNC).
     */
    public static final int OPEN_FLAG_SYNC = 1 << 20;

    private final @NonNull SerialPortInfo mInfo;
    private final @NonNull ISerialManager mService;

@@ -75,10 +126,9 @@ public final class SerialPort {
    }

    /**
     * Request to open the port. The flags must set
     * {@link android.system.OsConstants#O_NOCTTY}.
     * Request to open the port.
     *
     * Exceptions passed to {@code receiver} may be
     * <p>Exceptions passed to {@code receiver} may be
     * <ul>
     * <li> {@link ErrnoException} with ENOENT if the port is detached or any syscall to open the
     * port fails that come with an errno</li>
@@ -86,18 +136,15 @@ public final class SerialPort {
     * <li> {@link SecurityException} if the user rejects the open request</li>
     * </ul>
     *
     * @param flags     flags passed to open(2)
     * @param flags     open flags that define read/write mode and other options.
     * @param exclusive whether the app needs exclusive access with TIOCEXCL(2const)
     * @param executor  the executor used to run receiver
     * @param receiver  the outcome receiver
     * @throws IllegalArgumentException if the flags doesn't set {@link OsConstants#O_NOCTTY}, or
     *                                  any other parameters are {@code null}.
     * @throws IllegalArgumentException if the set of flags is not correct.
     * @throws NullPointerException     if any parameters are {@code null}.
     */
    public void requestOpen(int flags, boolean exclusive, @NonNull Executor executor,
    public void requestOpen(@OpenFlags int flags, boolean exclusive, @NonNull Executor executor,
            @NonNull OutcomeReceiver<SerialPortResponse, Exception> receiver) {
        if ((flags & OsConstants.O_NOCTTY) == 0) {
            throw new IllegalArgumentException("The flags must set OsConstants.O_NOCTTY");
        }
        Objects.requireNonNull(executor, "Executor must not be null");
        Objects.requireNonNull(receiver, "Receiver must not be null");
        try {
@@ -134,8 +181,7 @@ public final class SerialPort {
        private static Exception getException(int errorCode, int errno, String message) {
            return switch (errorCode) {
                case ErrorCode.ERROR_READING_DRIVERS -> new IOException(message);
                case ErrorCode.ERROR_PORT_NOT_FOUND -> new ErrnoException(message,
                        OsConstants.ENOENT);
                case ErrorCode.ERROR_PORT_NOT_FOUND -> new ErrnoException(message, ENOENT);
                case ErrorCode.ERROR_OPENING_PORT -> new ErrnoException(message, errno);
                default -> new IllegalStateException("Unexpected errorCode " + errorCode);
            };
+46 −7
Original line number Diff line number Diff line
@@ -16,6 +16,12 @@

package com.android.server.serial;

import static android.hardware.serial.SerialPort.OPEN_FLAG_DATA_SYNC;
import static android.hardware.serial.SerialPort.OPEN_FLAG_NONBLOCK;
import static android.hardware.serial.SerialPort.OPEN_FLAG_SYNC;
import static android.hardware.serial.SerialPort.OPEN_FLAG_READ_ONLY;
import static android.hardware.serial.SerialPort.OPEN_FLAG_WRITE_ONLY;
import static android.hardware.serial.SerialPort.OPEN_FLAG_READ_WRITE;
import static android.hardware.serial.flags.Flags.enableSerialApi;

import static com.android.server.serial.SerialConstants.DEV_DIR;
@@ -34,9 +40,11 @@ import android.os.RemoteCallbackList;
import android.os.RemoteException;
import android.system.ErrnoException;
import android.system.Os;
import android.system.OsConstants;
import android.util.Slog;

import com.android.internal.annotations.GuardedBy;
import com.android.internal.annotations.VisibleForTesting;
import com.android.server.SystemService;

import java.io.File;
@@ -50,6 +58,12 @@ import java.util.List;
public class SerialManagerService extends ISerialManager.Stub {
    private static final String TAG = "SerialManagerService";

    private static final int OPEN_MODE_BITS =
            OPEN_FLAG_READ_ONLY | OPEN_FLAG_WRITE_ONLY | OPEN_FLAG_READ_WRITE;
    private static final int FORBIDDEN_FLAG_BITS =
            ~(OPEN_FLAG_READ_ONLY | OPEN_FLAG_WRITE_ONLY | OPEN_FLAG_READ_WRITE | OPEN_FLAG_NONBLOCK
                    | OPEN_FLAG_DATA_SYNC | OPEN_FLAG_SYNC);

    // keyed by the serial port name (eg. ttyS0)
    @GuardedBy("mLock")
    private final HashMap<String, SerialPortInfo> mSerialPorts = new HashMap<>();
@@ -65,10 +79,7 @@ public class SerialManagerService extends ISerialManager.Stub {

    private final SerialDevicesEnumerator mSerialDevicesEnumerator = new SerialDevicesEnumerator();

    private FileObserver mDevDirObserver;

    private SerialManagerService() {
    }
    private SerialManagerService() {}

    @Override
    public List<SerialPortInfo> getSerialPorts() throws RemoteException {
@@ -119,7 +130,7 @@ public class SerialManagerService extends ISerialManager.Stub {
            }
            String path = DEV_DIR + "/" + portName;
            try {
                FileDescriptor fd = Os.open(path, flags, /* mode= */ 0);
                FileDescriptor fd = Os.open(path, toOsConstants(flags), /* mode= */ 0);
                try (var pfd = new ParcelFileDescriptor(fd)) {
                    callback.onResult(port, pfd);
                } catch (RemoteException | RuntimeException e) {
@@ -138,6 +149,34 @@ public class SerialManagerService extends ISerialManager.Stub {
        }
    }

    @VisibleForTesting
    static int toOsConstants(int flags) {
        // Always open the device with O_NOCTTY flag, so that it will not become the process's
        // controlling terminal.
        int osFlags = OsConstants.O_NOCTTY;
        switch (flags & OPEN_MODE_BITS) {
            case OPEN_FLAG_READ_ONLY -> osFlags |= OsConstants.O_RDONLY;
            case OPEN_FLAG_WRITE_ONLY -> osFlags |= OsConstants.O_WRONLY;
            case OPEN_FLAG_READ_WRITE -> osFlags |= OsConstants.O_RDWR;
            default -> throw new IllegalArgumentException(
                    "Flags value " + flags + " must contain only one open mode flag");
        }
        if ((flags & OPEN_FLAG_NONBLOCK) != 0) {
            osFlags |= OsConstants.O_NONBLOCK;
        }
        if ((flags & OPEN_FLAG_DATA_SYNC) != 0) {
            osFlags |= OsConstants.O_DSYNC;
        }
        if ((flags & OPEN_FLAG_SYNC) != 0) {
            osFlags |= OsConstants.O_SYNC;
        }
        if ((flags & FORBIDDEN_FLAG_BITS) != 0) {
            throw new IllegalArgumentException(
                    "Flags value " + flags + " is not a combination of FLAG_* constants");
        }
        return osFlags;
    }

    private void startIfNeeded() throws IOException {
        if (mIsStarted) {
            return;
@@ -148,7 +187,7 @@ public class SerialManagerService extends ISerialManager.Stub {
    }

    private void watchDevicesDir() {
        mDevDirObserver = new FileObserver(new File(DEV_DIR),
        FileObserver devDirObserver = new FileObserver(new File(DEV_DIR),
                FileObserver.CREATE | FileObserver.DELETE) {
            @Override
            public void onEvent(int event, @Nullable String path) {
@@ -166,7 +205,7 @@ public class SerialManagerService extends ISerialManager.Stub {
                }
            }
        };
        mDevDirObserver.startWatching();
        devDirObserver.startWatching();
    }

    private void enumerateSerialDevices() throws IOException {
Loading