Skip to content

Remove Null host - #1386

Open
Marco-Farruggio wants to merge 8 commits into
RustAudio:masterfrom
Marco-Farruggio:remove-null-host
Open

Marco-Farruggio wants to merge 8 commits into
RustAudio:masterfrom
Marco-Farruggio:remove-null-host

Conversation

@Marco-Farruggio

Copy link
Copy Markdown
Contributor

Remove the null host, add a compilation error on unsupported targets, and updated the documentation. To me expecting cpal to compile on a target with no audio capabilities is like expecting a GUI library to compile on a target with no display server, which should really be a build error. This also means its less clutter in the codebase which is always a plus.

Closes #1366

@Marco-Farruggio

Copy link
Copy Markdown
Contributor Author

Related: I think #571 can be closed because WASI doesn't support audio, and the proposed wasi-media isn't near starting

@LastExceed

Copy link
Copy Markdown
Collaborator

I have closed #571 . Think you can get CI to pass?

@Marco-Farruggio

Copy link
Copy Markdown
Contributor Author

Sorry, I only just saw this notification, and yes, I'll put some time aside tomorrow to get this ready to merge :)

@Marco-Farruggio

Copy link
Copy Markdown
Contributor Author

What do you think is a better idea for when there is no cpal host available for a certain compilation target and default_host() is called?:

  1. return the CustomHost, if its present, else emit a compile error stating there is no supported host, and maybe a useful message about the documentation and implementing a custom host.

  2. always emits a compilation_error!, even if CustomHost is present, in which case the user must explicitly use CustomHost (via host_from_id).

@Marco-Farruggio

Copy link
Copy Markdown
Contributor Author

Well it actually can't currently return CustomHost as CustomHost::new() is hardcoded to return an error, as it was expectedly to never be called

@LastExceed

Copy link
Copy Markdown
Collaborator

Just do whatever is easier right now, we can always change it later

@Marco-Farruggio

Copy link
Copy Markdown
Contributor Author

Why do we have CI checks for WASI when it's unsupported? They won't pass anymore as I emit a compilation error

@LastExceed

Copy link
Copy Markdown
Collaborator

No idea. Just remove them

@Marco-Farruggio

Copy link
Copy Markdown
Contributor Author

I think the WASM checks were actually all emitting false positives, as after removing the NullHost, it seems like that was all that was keeping them passing, I'm going to remove the WASI CIs now and see what I can do about the others (I haven't ever meddled with CIs before)

@Marco-Farruggio

Copy link
Copy Markdown
Contributor Author

Welp one removed null host later, a lot of comments, doc rewriting, and going down a CI rabbit hole for me, I think it's ready for merging 😅

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Null host shouldn't exist when a Custom host is present

2 participants