rhysmorgan134 / node-CarPlay

MIT License
147 stars 25 forks source link

Handle already initialised dongle in web #25

Closed steelbrain closed 1 year ago

steelbrain commented 1 year ago

This PR is about the new version of node-carplay in next branch for web version. Currently, even tho .reset() is called on the USB interface, it doesn't fully reset and keeps playing music if you reload the page with it.

What that means is that the video comes in, the audio comes in, but Plugged message does not. So there's two ways to handle this:

cc @gozmanyoni

Edit: The implications here would be that we don't send the pair message after X seconds of boot

gozmanyoni commented 1 year ago

My thoughts are that its better to figure out its plugged and streaming and just mark it as plugged - that way we recover from a reload faster, if the device is healthy and sending us data we just start using it as is

What do you think @rhysmorgan134?

rhysmorgan134 commented 1 year ago

Yeah I agree, feels the cleaner way to me to just have the data resume rather than a full reset.

steelbrain commented 1 year ago

Just to play the devil's advocate here, it's possible that the dongle already initialized doesn't match things like the wanted fps or the width/height, so it'll break expectations as the config will essentially be a no-op so isn't it a good idea to reset the dongle just to make sure the output is in the expected format?

rhysmorgan134 commented 1 year ago

I completely agree this needs to be looked into, the APK has a bunch of references to canvas size change and various linked functions, so I do think this can be done dynamically rather than requiring a reset. What do you think about going ahead without a reset and tracking the dynamic functionality in an issue, if it turns out not possible then perhaps revisit?

I think it would be great for a parent application to be able to perhaps switch to a 50/50 view and have CarPlay scale dynamically (layout of CarPlay changes based on window size)

gozmanyoni commented 1 year ago

That would be very interesting - I need to use the APK a bit and see what it sends over USB in these scenarios. I also want to add Multi Touch support - which seems fairly straightforward.

@steelbrain overall I agree - if possible we should have a mode where we restart an already started dongle. That's likely useful for local development rather than a production use-case where things wont change or restart after the link has started - perhaps a part of the driver config? and the examples could turn this option on

steelbrain commented 1 year ago

Just to add to the discussion, sometimes when resuming an already initialized dongle, I see a ton of Invalid buffer size - Expecting 16, got .... Seems non fatal, but haven't dug into why they occur yet.

gozmanyoni commented 1 year ago

That's an error I added - if we read data from the usb dongle and its not header data it will throw - after a few errors it will close the driver - its probably raw audio or video data its getting as its trying to read

steelbrain commented 1 year ago

the APK has a bunch of references to canvas size change and various linked functions

It would be very interesting to explore the possibility of adaptive frame rates, if the device is fighting to handle it, we could keep lowering the FPS until it is stable

gozmanyoni commented 1 year ago

@steelbrain indeed - i wonder if its possible to adjust it without resetting the connection.

I do think its quite a nice-to-have feature rather than a crucial one - the usage of this is on car interfaces, where the consumer will usually fine-tune it to something that works well in all cases

steelbrain commented 1 year ago

Closing since a PR addressed it