Allow to open Master with read-only access

[flosse]

Motivation

I tried to periodically receive PDOs and execute the Master::sdo_upload method once in a while but that leads to a totally blocked process (zombie process). I have no idea why.

Then I tried to do the cycles with my Rust application and manually read/write SDOs via the CLI (e.g./opt/etherlab/bin/ethercat download -t float 0x8022 1 10.9) and it worked fine.

So I had a look at the CLI code (CommandUpload.cpp) where I found this line that creates the access:

m.open(MasterDevice::Read);

which executes this code (tool/MasterDevice.cpp):

void MasterDevice::open(Permissions perm)
{
    stringstream deviceName;

    if (fd == -1) { // not already open
        ec_ioctl_module_t module_data;
        deviceName << "/dev/EtherCAT" << index;

        if ((fd = ::open(deviceName.str().c_str(),
                        perm == ReadWrite ? O_RDWR : O_RDONLY)) == -1) {
            stringstream err;
            err << "Failed to open master device " << deviceName.str() << ": "
                << strerror(errno);
            throw MasterDeviceException(err);
        }

        getModule(&module_data);
        if (module_data.ioctl_version_magic != EC_IOCTL_VERSION_MAGIC) {
            stringstream err;
            err << "ioctl() version magic is differing: "
                << deviceName.str() << ": " << module_data.ioctl_version_magic
                << ", ethercat tool: " << EC_IOCTL_VERSION_MAGIC;
            throw MasterDeviceException(err);
        }
        masterCount = module_data.master_count;
    }
}

This looks similar to the reserve method in the rust wrapper:

    pub fn reserve(index: MasterIndex) -> Result<Self> {
        let devpath = format!("/dev/EtherCAT{}", index);
        let file = OpenOptions::new().read(true).write(true).open(&devpath)?;
        let mut module_info = ec::ec_ioctl_module_t::default();
        let master = Master {
            file,
            map: None,
            domains: HashMap::new(),
        };
        ioctl!(master, ec::ioctl::MODULE, &mut module_info)?;
        if module_info.ioctl_version_magic != ec::EC_IOCTL_VERSION_MAGIC {
            Err(Error::new(
                ErrorKind::Other,
                format!(
                    "module version mismatch: expected {}, found {}",
                    ec::EC_IOCTL_VERSION_MAGIC,
                    module_info.ioctl_version_magic
                ),
            ))
        } else {
            ioctl!(master, ec::ioctl::REQUEST)?;
            Ok(master)
        }
    }

The main difference is this line:

ioctl!(master, ec::ioctl::REQUEST)?;

Solution

My solution was to add a open method to the Master that does not contain the line ioctl!(master, ec::ioctl::REQUEST)?; and now I'm able to create another Master instance within an other thread where I can call Master::sdo_upload that actually works :)

Open Questions

• Is this PR a good idea in general?
• how could the open method look like?
  • open(idx: MasterIndex, write_access: bool)
  • open(idx: MasterIndex, access: MasterAccess) with enum MasterAccess { ReadOnly, ReadWrite }
  • open_ro(idx: MasterIndex)
  • open_rw(idx: MasterIndex)
• What should be returned?
  • Result<Master>
  • Result<(Master, usize)> to get also module_data.master_count?

[birkenfeld]

Looks like we should definitely support this. However, there seems to be two differences here:

• Calling the REQUEST ioctl on the master

According to the header docs, this is necessary for registering domains/PDOs and realtime access.

• Opening the device with MasterDevice::Read or MasterDevice::ReadWrite (in the C++ terminology in tool/)

Commands like upload only need read access, while e.g. download needs read-write. REQUEST also needs read-write.

So IMO either we do enum AccessType { ReadOnly, ReadWrite, Realtime } or we separate the open(Permission) and request() operations like you did in the PR.

Regarding master_count, it should probably be a static method that opens its own FD, just to query this, like in the C++ code (see tool/Command.cpp, line 50.)

[flosse]

So IMO either we do enum AccessType { ReadOnly, ReadWrite, Realtime } or we separate the open(Permission) and request() operations like you did in the PR.

One the one side I like the idea of a simple API with a single open method but probably there are scenarios where you might need a better control?

Currently my favorite solution is

• an open(index, permission) method with either MasterAccess::ReadOnly or MasterAccess::ReadWrite
• and a separate reserve() method (like in lib/master.c line 45) to get realtime functionalities.

Regarding master_count, it should probably be a static method that opens its own FD, just to query this, like in the C++ code (see tool/Command.cpp, line 50.)

:+1: