-
Notifications
You must be signed in to change notification settings - Fork 92
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
Whitespace Killer #82
Conversation
### Mapped Unused Keys: - Call Answer: Phone Screen (Answer Call) - Call End: End Call - Fav - AA Home Screen - Nav - Attempt to take back video focus + fix trailing spaces
So I can use ⭐️ to reference FAV when explaining how to gain back video focus to users. Plus seems more natural to have button functions like that.
@@ -477,7 +481,7 @@ | |||
videoConfig->set_margin_width(0); | |||
videoConfig->set_margin_height(0); | |||
videoConfig->set_dpi(140); | |||
inner->set_available_while_in_call(false); | |||
inner->set_available_while_in_call(true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this change intended? It seems like currently due to the video focus stealing, it's not available in a call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok so that is the one thing I was kindof unsure on because my mind thinks if I set that to true it will make it available during the call and false will cause it to not be available, so guaranteed loss of video focus. but what does that field actually do?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly I'm not sure 100% since we are reverse engineering this, but since this is the data structure describing the car to the phone and this represents a output stream the car is providing, I assumed this to mean "Does a phone call through BTHF always override/ignore this output regardless of focus?" In the current setup were we lose video and audio focus in a call it seems the answer is false. However if we fix it then probably true for video (since we want the AA UI) and false for USB audio since the car force switches to BT. But this is just my understanding currently, it could be wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've had it set to true in my own tests and I tried false too I saw no difference but I figured that true is better because that's what we want in the end, for video to be available during the call so true is a better choice regardless if it works or not because false can only mean something that we ultimately don't want. right? does that sound reasonable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I guess I can see the argument, especially till we get stuff completely working with the calls, my interpretation was that is was an optimization like "don't bother sending this data while in a call since the car will ignore it" which might explain why it seems to do nothing. In that case you should probably set it on the AA_CH_AU1 output also.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You know if you put AA in developer mode then calls will show up even if you aren't connected to BT. This leads me to believe that there is also a way to route the call through USB if you are in developer mode.
I understand what you are saying but I was kind of talking about a middle ground between what we have now and what you are saying and by that I mean not to disable BTHF completely but just to override the call answering and ending functions in particular meaning that BTHF could still run and handle certain things that it handles but we write some code in headunit that overrides and handles the functions AcceptCall() and HangupCall() or whatever else we write into it instead of BTHF. Does that make sense and is it possible? I think it is to just overwrite handling of certain functions without disabling the whole service right? And I am pretty sure that using the CallStatus signal handler will allow up to do this to a certain extent. Of course this will not at all be a trivial task to accomplish.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You know what I just thinking about this, someone came up with a workaround to the bluetooth call bug by doing this:
- set AA to developer mode so you can use the AA call interface
- use tasker or something similar to auto speakerphone when AA is running
- turn off bluetooth on phone and car so both dont think they are connected (and they arent).
Now the interesting thing about this is that you would NOT lose video focus and I don't think BTHF would have any part in handling the call at all. During a call the AA call interface worked perfectly like it was connected and everything the only thing wrong is that the sound would be coming from the phone on speakerphone instead of the speakers. Now using this idea if we can find that magic USB phone call route that I am pretty confident is at least possible in some sense then that may be the ticket.
But the first Idea about just overriding a few particular BTHF methods and/or signal handlers seems like a little better and more viable solution I'm just throwing some more ideas into the convo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I dunno it's possible, but complex. I think the path of least resistance, if it's possible, would be to filter or snoop the dbus messages on the com.jci.nativeguictrl
interface, specifically SetRequiredSurfaces. We are emitting this signal to change the video focus, but also it's what everything else on the car uses AFAIK so if we can intercept these messages we can selectively ignore them to keep video focus during the call, or at least see when it's changed so we can change it back. That one thing seems like it would make the entire existing BTHF mode work right?
I'm not sure how to go about snooping/filtering a dbus interface since everything I've learned about dbus I learned working on this, but so far it seems over-engineered enough it may be possible 😆
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quick googling makes it seem like it's possible and probably the C++ wrapper we are using also has a version of dbus_connection_add_filter()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lmagder I've come up with the same idea to sniff these d-bus method calls and wrote a quick PoC here mishaaq@e05db16, but haven't tried it yet. Additionally tried BTHFClient::CallStatus signal handing idea by @Trevelopment to properly release/request video focus on call.
@@ -492,6 +496,7 @@ | |||
audioConfig->set_sample_rate(48000); | |||
audioConfig->set_bit_depth(16); | |||
audioConfig->set_channel_count(2); | |||
inner->set_available_while_in_call(true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought maybe if I set this to true it would override BTHF and take over sound handling but in testing it has no effect on anything
@@ -142,7 +142,7 @@ namespace nmea { | |||
Event() : enabled(true) | |||
{} | |||
|
|||
virtual ~Event() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The files in the nematode folder are a 3rd party lib, we probably shouldn't reformat them incase we want to merge a new version later, it comes from here: https://github.com/ckgt/NemaTode
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok good point
@@ -7,7 +7,7 @@ Name: Protocol Buffers | |||
Description: Google's Data Interchange Format | |||
Version: 2.6.1 | |||
Libs: -L${libdir} -lprotobuf -pthread -lpthread | |||
Libs.private: -lz |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also a 3rd party file
@@ -42,7 +42,7 @@ extern "C" { | |||
require recompiling all users of this library. Stack allocation is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same with libunwind headers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok i just went to crazy with it i opened every file and pressed save all hahaha
eee4492
to
4bc47ce
Compare
no more 3rd partys |
This is just all white space fixes, a white space holocaust in a sense...