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

Device autodetection #45

Open
opeik opened this issue Dec 4, 2023 · 9 comments · May be fixed by #49
Open

Device autodetection #45

opeik opened this issue Dec 4, 2023 · 9 comments · May be fixed by #49
Labels
enhancement New feature or request

Comments

@opeik
Copy link

opeik commented Dec 4, 2023

Hi there,

Does this wrapper support autodetecting a Pulse Eight CEC Adapter? I'm fiddling withlibcec-sys to hopefully make it work.

The example cec-client autodetects devices if one is not specified.

@opeik opeik added the enhancement New feature or request label Dec 4, 2023
@opeik opeik changed the title Autodetection Device autodetection Dec 4, 2023
@opeik
Copy link
Author

opeik commented Dec 4, 2023

This lovely hack for CecConnectionCfg::open appears to work:

    pub fn open(mut self) -> CecConnectionResult<CecConnection> {
        ...
        let mut devices: [libcec_sys::cec_adapter_descriptor; 10] = unsafe { std::mem::zeroed() };
        let num_devices = unsafe {
            libcec_sys::libcec_detect_adapters(
                connection.1,
                &mut devices as _,
                10,
                std::ptr::null(),
                true as i32,
            )
        };

        if num_devices < 0 {
            panic!("no devices found")
        }

        if unsafe { libcec_open(connection.1, devices[0].strComName.as_ptr(), open_timeout) } == 0 {
            return Err(CecConnectionResultError::AdapterOpenFailed);
        }
        ...
    }

@ssalonen
Copy link
Owner

ssalonen commented Dec 5, 2023

Rusty wrapper for detection has not been implemented, no. You can find it in the list of unimplemented functions here:

cec-rs/src/lib.rs

Line 1282 in 2ada364

// extern DECLSPEC int8_t libcec_detect_adapters(libcec_connection_t connection, CEC_NAMESPACE cec_adapter_descriptor* deviceList, uint8_t iBufSize, const char* strDevicePath, int bQuickScan);

Currently port name needs to be supplied by the user.

Pull requests and API design are most welcome :)

@opeik
Copy link
Author

opeik commented Dec 18, 2023

I made it work, the timeline went:

  • Failed to build cec-rs due to libcec failing to build
  • Failed to build libcec manually
  • Reversed the libcec build process
  • Fixed the libcec-sys build script
  • Realized libcec-sys rebuilds on every change which causes a rebuild of libcec taking about 10 minutes
  • Rewrote the build script to use precompiled binaries
  • Realized cec-rs's use of #[repr(u32)] on enums breaks the windows build due to differences in C enum representations on MSVC
  • Fixed the enums
  • Cried

Anyhow, my fork diverged substantially and I'm unsure how to upstream it.

Links:

@ssalonen
Copy link
Owner

It is sad to read how painful the windows developer experience is here. This reveals the current setup is not yet very polished/production grade even though the build and ci passes in github with windows.

Similar CI/CD optimizations have been discussed/proposed in ssalonen/libcec-sys#45 although not sure if the person will follow it through

This is first time I hear incompatibility with msvc, can you file ticket about it, please? How does it surface? I can see how this lead to deeper rabbit whole and introducing the bindgen as part of the build loop (as opposed to static runs of bindgen + python like now)...

re upstreaming: indeed, sounds like this needs quite a bit of work to split into manageable and logical chunks

@opeik
Copy link
Author

opeik commented Dec 19, 2023

Similar CI/CD optimizations have been discussed/proposed in ssalonen/libcec-sys#45 although not sure if the person will follow it through

I ended up using the precompiled binaries mentioned in that issue.

This is first time I hear incompatibility with msvc, can you file ticket about it, please? How does it surface?

As far as I can tell, I:

  • Installed VS2019 as per the instructions
  • Ran bindgen on the vendored/precompiled cecc.h with the same options as bindgen.sh
  • Compared the output with the committed bindings and observed the constified enums were i32 instead of u32

I can see how this lead to deeper rabbit whole and introducing the bindgen as part of the build loop (as opposed to static runs of bindgen + python like now)...

In my experience, running bindgen in the build script is far less annoying and surfaces issues faster compared to committing the bindings.

@ssalonen
Copy link
Owner

ssalonen commented Dec 26, 2023

Compared the output with the committed bindings and observed the constified enums were i32 instead of u32

Did this somehow generate build errors or runtime errors? What makes it really "incompatible"?

As far as I can understand, both types are compatible with the enum values. Some enums are signed when the c type contains negative values.

Re. Precompiled binaries

Instead of 3rd party precompiled binaries, I would instead prefer to use the dll's distributed via the official installer.

The approach details were discussed in the same issue

@opeik
Copy link
Author

opeik commented Jan 19, 2024

Did this somehow generate build errors or runtime errors? What makes it really "incompatible"?

It was build errors. From memory it was incompatible because u32 can't be losslessly cast to i32.

@ssalonen
Copy link
Owner

Coming back to this...

As far as I can tell, I:

Installed VS2019 as per the instructions
Ran bindgen on the vendored/precompiled cecc.h with the same options as bindgen.sh
Compared the output with the committed bindings and observed the constified enums were i32 instead of u32

I understand you might get build failures if you change the signatures...

But what the very first build error is MSVC, before you changed any of the code?

@opeik
Copy link
Author

opeik commented Jul 20, 2024

@ssalonen Hey, sorry for the delay. The first set of build error was with your code, untouched by me. If you make a dummy app that depends on cec-rs on Windows it should fail to build.

@kira-bruneau kira-bruneau linked a pull request Sep 12, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants