BimmerGestalt / AAIdrive

Implementations of some Android Auto features as unofficial IDrive apps
MIT License
552 stars 90 forks source link

PhoneUI improvements #81

Closed hufman closed 3 years ago

hufman commented 4 years ago

The PhoneUI is intimidating for new users, who are dropped into the app without any introduction or explanation. @bogdan-calapod has some good ideas on ways to fix it.

bogdan-calapod commented 4 years ago

Hey there 😁

I've attached a zip file containing all the exported assets and exported PNG images of each of the screens, as well as the .xd file and a screen recording of the user flow through the app.

πŸŽ₯ XD file & recording - App Mockup.zip πŸ“ Assets - AndroidAutoIDrive.zip

bogdan-calapod commented 4 years ago

I'm trying to get started with the mockups again and make a complete flow, but I'm a bit swamped with time these days :(

So what I'm thinking is to make a basic overview of the screens that we'll need, and start from there.

I'm thinking:

Also what way do you think is good for storing these design files ? I usually keep them in my OneDrive and can share a link to the files, so that everyone can see the latest version there ?

hufman commented 4 years ago

It's all good! I've been busy working on other parts of the app, and still have maybe another month before I'm ready to start tackling the phone UI πŸ‘ That's a good list, all the most important parts for users! I was thinking of organizing it with a hamburger side menu instead of tabs to match the Connected app, and to easily support a bigger list of main features:

I do plan on remembering the information about the last car, to show information about the supported features in the relevant tabs (greying out Popups if the current/last car is IDrive5+, for example). I'm not sure about showing the same information about the official Connected app, since it also caches most of that information, but perhaps a default-hidden tab for more advanced details might be fun for other nerds! (Unless, does Connected not show that information if the ConnectedDrive subscription isn't live? That might be a reason to add that info)

Interesting thought about a list of destinations to open with Google Maps! I'm not sure how it would work differently than Google Maps' own recent list. I hope that #131 should be automatic enough to not need any options, what are you thinking about?

As for the resources, attaching screenshots and zip files seems to be a good workflow to me, if you don't want to use up your OneDrive space for historical purposes?

Thank you so much for working on this!

bogdan-calapod commented 4 years ago

I was thinking of organizing it with a hamburger side menu instead of tabs to match the Connected app, and to easily support a bigger list of main features

Agreed - I think it makes more sense, and makes my life easier as well πŸ˜…

Interesting thought about a list of destinations to open with Google Maps! I'm not sure how it would work differently than Google Maps' own recent list. I hope that #131 should be automatic enough to not need any options, what are you thinking about?

I was thinking that you could auto-launch Google Maps from the car to a certain navigation - I think it wasn't clear enough that I was talking about the car UI πŸ˜ƒ If we could pull the recent places from Maps and display them in the car, then trigger the navi (e.g. Google Maps navi, not the in-car system if it exists) from there, that would be awesome. If not - the user could define a list of "favorite places" and we could display that if no active navigation (e.g. Google Maps notification) is present.

As for the resources, attaching screenshots and zip files seems to be a good workflow to me, if you don't want to use up your OneDrive space for historical purposes?

I can store them on OneDrive no problem - I already store them there anyways πŸ˜ƒ

Thank you so much for working on this!

Thank you so much for making this project a thing - it really shows the full power of what can be done with dedication and interest! πŸ’ͺ

berseker commented 3 years ago

About this subject, It would be Nice to add an option to mirror the main menu on the right part of the screen, for an Easy use with the right hand only (sometimes in the car you May Need to touch something in the settings.. i know that while driving the phone shall not be used in any case, but i drop this idea here anyway :)

I also look forward to the "car info" Tab, It would be a Nice feature to add

bogdan-calapod commented 3 years ago

@berseker you mean adding an option to flip the sidebar to come from the right side ?

I think that's easy to implement, although a bit unorthodox WRT Android's UX guidelines. What I've seen most apps migrate to is having the main actions in a bottom bar (something similar to what new-ish apps like Google Maps have done recently)

berseker commented 3 years ago

@bogdan-calapod exactly. by looking at this link the positioning is "recommended" only, and it is allowed specifically for right-to-left languages https://material.io/components/app-bars-top#anatomy

also a bottom bar could be a nice & more usable alternative in my opinion ;)

hufman commented 3 years ago

What settings do you find yourself needing to change while driving? I intend the phone interface to only be needed for initial setup, with the car interface providing any additional quick options like the Notification settings.

I opted for the slide-out menu because of the clearer presentation of the number of features in the app, with each major area of functionality having its own dedicated tab. The guidelines suggest that a bottom navbar is only for a very limited number of options (3-5 per the docs). There was also the idea of behaving similarly to the Connected app, which was also using a sidebar (granted, MyBMW now has a 3-item bottom navbar heh).

The home screen should show you some basic information about your car and a list of app features that your car supports. If you enable Advanced Options, a new Connection tab will be present (and clicking the connection status icons will also bring you here). With the Advanced Options enabled, it will show you some extra detailed information about your car, such as the full list of car capabilities and some selected data points from the car. What additional car information would you like to show?

berseker commented 3 years ago

I found a bit difficult to reach the main menu during drive since it is placed on the left. that was only my suggestion, to put an option to "mirror" it to the right part of the screen. but I also understand that some Android guidelines must be followed to prepare an app interface.

about the "bottom bar", I think that a main menu of 5 items could be used. I'll try to work on a mockup in order to show you some ideas :D but in any case also the interface like it is , it's ok I think

about the "additional car infos" I was talking about the "Vehicle info screen" mentioned by @bogdan-calapod on 26 October, with some "connected drive app" infos like car range, remaining fuel, but also oil temperature and other "semi-hidden" infos on BMW like some dedicated apps like bimmerlink or carly do (see a sample screenshot below) image If i'm not mistaken, your app does extract these infos from the connection and already shows some of them in the "connection tab"

berseker commented 3 years ago

@hufman I made a mockup of my idea.. please note that this is only a brainstorming that maybe could be useful to improve the (already fantastic) app, if some concept expressed are not feasible or simply against Android interface rules\other "guidelines" you are trying to follow in the interface, feel free to label this mockup as "not useful\trash" :D image

ps maybe point 4 of my mockup could be dedicated to some sort of "Vehicle info screen" as hypotized also by @bogdan-calapod in October

bogdan-calapod commented 3 years ago

@berseker the point of the mockup is just what you did - make people understand what you mean πŸ˜„ So I think your mockup is very valuable!

I can try to play a bit in XD this weekend, see if I can draw something up along the lines you presented - there are some valid and nice points (the panoramic dashboard view I think is a really cool idea!)

berseker commented 3 years ago

about that dashboard, I think that the best could be replicate if possible some sort of image like that mockup also directly on the car screen :D

hufman commented 3 years ago

With BMW Connected going away and MyBMW not supporting ID4 cars without Remote Data access, I think it makes more sense now to add a Car Information tab to show some of the previously available information (odometer, fuel level, etc).

jezikk82 commented 3 years ago

MyBMW doesn't support ID5 cars with subscription as well. Please consider to also to cache some of this values (odometer, fuel level, etc). in app, so you can view them without connection

hufman commented 3 years ago

Definitely possible, the work around bringing CDS values into the PhoneUI unlocks this easily. For example, the car's language shown in the Settings is a cached CDS value. The CarInformationUpdater class would just need to register for more data points to be available all the time as opposed to the data points in the Connection tab that are only updated while the tab is visible. Then a new Car Information tab will always have access to these values, even when not connected.

hufman commented 3 years ago

I posted an updated build of the first_start branch, I'd appreciate feedback on this concept!

berseker commented 3 years ago

Very nice welcome screens! I would add some "images" on every screen in order to fill up the empty space ( I do not know if you can specify only on High res screens) in any case very good job! I really like the "unbranded brand" icons πŸ˜ƒ

fabssor commented 3 years ago

I Like it very much but I want to share the following thoughts.

I am no laywer but I do not think the Mini or BMW Logos should be used inside the app. I think this ist even more important If the release to the app store should happen (#212). There might be some Copyright issues with the Logo and maybe even with the name IDrive.

Just wanted to leave this here as a side note.

hufman commented 3 years ago

It's hard to say for certain, and different regions have different laws about it. There's precedent for the companion apps like Spotify showing a splash screen displaying the official brand icon, so I believe that including a reminiscent but different icon is safe. The iDrive name is similar: The official branding for companion apps is ConnectedDrive, with IDrive only being the headunit system itself. Some examples of fair use seem to apply, such as referring to a company's trademark without claiming to be endorsed or provided by it.

BMW does know about the project (has reached out and said it was neat) yet hasn't taken any sort of action towards it. I think by clearly indicating its unofficial nature, any trademark infringement is avoided, and users aren't confused into thinking it's an official app and demanding official support from BMW.

hufman commented 3 years ago

I posted a branch with a new Car Information tab, check out it out!

jezikk82 commented 3 years ago

Tested new car info tab and works & look fine. Thanks for great work.

Based on my previous testing I'd add following info: 1. CDS.NAVIGATION.CURRENTPOSITIONDETAILEDINFO to "currentPositionDetailedInfo", => address, work ok only in big cities, on rural areas often show only generic info about countryside you are in. That why both the info are needed & useful. CDS.NAVIGATION.GPSPOSITION to "GPSPosition", => exact coordinates -> link to GMAP CDS.NAVIGATION.GPSEXTENDEDINFO to "GPSExtendedInfo" => altitude with link to GMaps. Very useful in foreign cities when you look where you park your car.

  1. CDS.ENGINE.TEMPERATURE to "temperature", => engine & oli temp.
  2. CDS.DRIVING.AVERAGECONSUMPTION value for second computer + additional data (average speed, distance, time)
  3. CDS.SENSORS.BATTERY to "battery" in petrol cars it shows the battery charge level and indirectly condition when fully charged. If possible cache the max value reported and present after actual in brackets.
  4. CDS.DRIVING.PARKINGBRAKE to "ParkingBrake" - it's sometime not reported by car:/ but I know it useful for women :)
  5. CDS.DRIVING.SPEEDACTUAL (CDS.DRIVING.SPEEDDISPLAYED)
  6. Add revision of the i-level of the car: it in the line NBTevo_ID5_1911Release ID51911-490-1922L
    • 1911 - is the main i-level of the car
    • 490-1922L is the actual SW version

Optionally, live car stats: CDS.ENGINE.TORQUE to "torque", CDS.ENGINE.RPMSPEED to "RPMSpeed", CDS.DRIVING.GEAR to "gear", // I can prepare mapping if you dont have. CDS.DRIVING.ACCELERATION to "acceleration", CDS.DRIVING.ACCELERATORPEDAL to "acceleratorPedal", CDS.SENSORS.TEMPERATUREINTERIOR to "temperatureInterior", CDS.SENSORS.TEMPERATUREEXTERIOR to "temperatureExterior",

jezikk82 commented 3 years ago

Error: After longer ride i noticed the range on petrol is not refreshing as rest of data. I got new odometer reading, last update date but range not.

Moreover, we can add one more value: Last trip distance, where we calculate last odometer value from engin start time and substruct it from actual value.

As a next step we could visualise the reading (especially: rpm, gear, engin/oil, acceleration/brake, g-meter in a FIA rally style or Aventador). That would be awesome. Everything with live data.

jezikk82 commented 3 years ago

@hufman quick questions:

  1. How Can I read unchaced data in CarDrivingStatsMode.kt? Now I have something like that but unfortunatelly it return null: val speedActual = carInfo.cdsData.liveData[CDS.DRIVING.SPEEDACTUAL].map { it.tryAsJsonObject("speedActual")?.tryAsJsonPrimitive("speedActual")?.tryAsDouble }.format("%.0f").addUnit(unitsAverageSpeedLabel)

  2. Where I can find temperature because CDS.VEHICLE.UNITS to "Units", in CarAdvanceInfoFragment.kt is returning Null for Me :/ If you have full units json please sent and let me know why I got null answer ?

  3. Have you seen values for Driving Computer 2 for distance and time ?

  4. We have Torque in the cds values, but is the also actual engine power used (horse power) (In sport displays in HMI we home engine power and torgue)

I'll try to prepare some update and do PR with some of the point mentioned above. Already done point 2,3,4 and started to work on 5-6 and additional, but I failed as I cant read real values as shown above.

hufman commented 3 years ago

Good morning! Thanks for the feedback, I'll incorporate it soon into the branch, planning to merge CarDrivingStatistics with only the after-drive info implemented to clear the way for you to add the live statistics. I don't have good design sense, so I will leave the gauge displays up to you :)

The petrol range is intended to be updating, I'll take a look. For your SpeedActual null, you can skip the tryAsJsonObject step: This is for something like, say, GPSPosition, where it would tryAsJsonObject("GPSPosition")?.tryAsJsonPrimitive("latitude"). An example Units json is found on this page, and with the knowledge that this was taken from an American car built in Germany, you can figure that the sample temperature:2 units mean Fahrenheit and that 1 would be the sensible Celcius units. The Units block definitely shouldn't be null! I just hadn't implemented parsing the Temperature units because of the after-drive type of data I wanted to show in this first implementation. Also on that page you can see the Driving Computer 2, I haven't seen any valid results for Computer 2 and perhaps my Mini just doesn't implement it. I haven't seen a horsepower data point. Perhaps it's somehow calculated from the acceleration, accelerator pedal, and gear positions? I found this M Laptimer app which I should look at to see what it's doing, but I haven't found an Android version of the old Mini Sports Displays app

jezikk82 commented 3 years ago

M LapTimer was actually what had in my mind asking for engine power and proposing some gauges like FIA. However, I don't think M Lap Timer was for Mini, It was dedicated only to M3/M5 cars and spreed across all BMW M variants. It was not working with regular cars.

SpeedActual I'll test with this syntax: val speedActual = carInfo.cdsData.liveData[CDS.DRIVING.SPEEDACTUAL].map { it.tryAsJsonPrimitive("speedActual")?.tryAsDouble }.format("%.0f").addUnit(unitsAverageSpeedLabel)

hufman commented 3 years ago

I pushed up a new build incorporating a bunch of your feedback: IDrive Version and GPS coordinates are now shown in Car Info, if Advanced Settings is enabled. GPS Address is shown if known, and is clickable to open any map app. Added the Acc Battery and Average Consumption 2 values if they look valid. Fixed the fuel range calculation. Adds Temperature unit parsing.

The other values (temperatures, parking brake, speed actual vs displayed) would fit better as live gauges, I think.

Check it out and let me know!

jezikk82 commented 3 years ago

I tested this with some extra addition I made yesterday on the way for around 100km. Work without problem. I'm still fighting against for returning values of driving.mode, driving.parkingBrake, controls.sunroof. Moreover, I'll try to add compass feature based on the gps heading value.

I'll create PR with my changes later this week and after I think we can add it to main, but it's up 2 you.

hufman commented 3 years ago

Feel free to add it in a new tab too, to give the new information lots of room to show off! I imagine this Car Information tab is for more static information and after-parking info, with a separate tab/fragment/viewmodel dedicated to the more dynamic visualizations.

jezikk82 commented 3 years ago

For now the only "real time data" are: actual speed, driving gear, driving mode (ongoing), compass and Engine & oil temp. I moved that info on top of the page. Data like RPS, Torque, acceleration need to be visualized in some king of gauges, but I'm unfortunately not gfx designer :(

Below are more static info. I'll share screen later to present what I have.

p.s. Can someone check if that works for You: 1) val parkingBrake = carInfo.cdsData.liveData[CDS.DRIVING.PARKINGBRAKE].map { it.tryAsJsonPrimitive("parkingBrake")?.tryAsInt }

2) val drivingMode = carInfo.cdsData.liveData[CDS.DRIVING.MODE].map { val a = it.tryAsJsonObject("mode")?.tryAsJsonPrimitive("mode")?.tryAsInt val b = it.tryAsJsonPrimitive("mode")?.tryAsInt "DEBUG: $a - $b" }

3) val sunRoof =carInfo.cachedCdsData.liveData[CDS.CONTROLS.SUNROOF].map { val status = it.tryAsJsonObject("sunroof")?.tryAsJsonPrimitive("status")?.tryAsString?.takeIf { it !="" } ?: " " val openPosition = it.tryAsJsonObject("sunroof")?.tryAsJsonPrimitive("openPosition")?.tryAsString?.takeIf { it !="" } ?: " " val tiltPosition = it.tryAsJsonObject("sunroof")?.tryAsJsonPrimitive("tiltPosition")?.tryAsString?.takeIf { it !="" } ?: " " "DEBUG: Status: $status | Open: $openPosition | Tilt: $tiltPosition" }

Please share results JsonObject with comment. In my car it returns null always, weird....

jezikk82 commented 3 years ago

Screenshot from the app after my changes: image image

work ongoing...

hufman commented 3 years ago

Thanks, that looks great! One option is to have separate TextViews to show the speed and the speed units with different sizes. For example, the totalRange LiveData is a number, and it's combined with the unitsDistanceLabel LiveData to create the totalRangeLabel combined label for simplicity, but there could be a currentSpeed numeric LiveData, with a separate TextView showing just unitsAverageSpeedLabel at a smaller size. This also enables currentSpeed to be a basic number to be used for future gauge displays, maybe!

I tested out your code, and it works pretty well for me; image

The Sunroof needed me to use tryAsInt instead of tryAsString, and CDS.CONTROLS.SUNROOF needed to be added to the CACHED_KEYS at the top so that the background service knows to load cachedCdsData with that data. The status was 0 when closed, tilt changed to 5 when it was opened (perhaps meaning 5 degrees? I only have a single closed/tilted state, perhaps other cars have multiple tilt positions), and then "open" seemed to indicate how open the roof was, saying 0 when just tilted open and 45 when the roof was as open as it could go in my car. The status field changed to 1 when the root went more open than just tilted.

My parking brake only showed 1, in my current car with an electronic brake, either engaged or disengaged. My dashboard shows a Red PARK word on or off depending on the parking brake, and it has the light to show a Yellow BRAKE word. The car that I captured the data on for the CDS example page had a hand brake and showed 2 at that time. From looking in the BMW Connected app code (in CdsTypes.EParkingBrake2), the values seem related to which brake indicator lights are displayed: 0 - Bremsensymbol_AUS 1 - Bremsensymbol_Gelb 2 - Bremsensymbol_Rot 3 - Bremsemsymbol_Gruen 4 - PARK_Gelb 8 - PARK_Rot 12 - Park_Gruen 16 - AutoP_Gelb 32 - AutoP_Rot 48 - AutoP_Gruen 127 - Unknown

My car shows a driving.mode of 3 (via the second conversion line of your code, without tryAsJsonObject), and didn't change depending on whether I had the car in accessory mode or actually running. It did change for Sport mode (4) and Eco mode (7). The CdsTypes.EVehicleMode shows the following constants: 0 - Initialisierung 1 - Tractionmodus 2 - Komfortmodus 3 - Basismodus 4 - Sportmodus 5 - Sportmodusplus 6 - Racemodus 7 - Ecopro 8 - Ecoproplus 9 - Komfortmoduserweitert 15 - Unknown

My car's JSON looks like this, today:

controls.sunroof = {"sunroof":{"openPosition":0,"tiltPosition":0,"status":0}}
driving.mode = {"mode":3}
driving.parkingBrake = {"parkingBrake":1}

I just pushed up a change to CarInformation to allow the cachedCdsData objects to be updated faster while they are on screen. This has the possibility of confusion, for example the added Sunroof data would work without needing to add it to the CACHED_KEYS, but it would only update the cached value while the Car Info tab is viewed. Edit: Actually, I think this confusion won't happen after thinking more: It will register for faster updates when the cachedCdsData.liveData becomes visible, but only incoming car data that is found in the CACHED_CDS_KEYS gets updated in cachedCdsData. So, the liveData will stay null until it is set in CACHED_CDS_KEYS.

jezikk82 commented 3 years ago

In my case the following code: val parkingBrake = carInfo.cachedCdsData.liveData[CDS.DRIVING.PARKINGBRAKE].map { it.tryAsJsonPrimitive("parkingBrake")?.tryAsInt } cause force close of the app. I sent you report from app.

Short snippet: java.lang.RuntimeException: Failed to call observer method at androidx.lifecycle.ClassesInfoCache$MethodReference.invokeCallback(ClassesInfoCache.java:226) .... Caused by: android.content.res.Resources$NotFoundException: String resource ID #0x0 at android.content.res.Resources.getText(Resources.java:381) at android.content.res.MiuiResources.getText(MiuiResources.java:97) at android.widget.TextView.setText(TextView.java:6397) at me.hufman.androidautoidrive.databinding.CarDrivingStatsBindingImpl.executeBindings(CarDrivingStatsBindingImpl.java:1398) at androidx.databinding.ViewDataBinding.executeBindingsInternal(ViewDataBinding.java:473) at androidx.databinding.ViewDataBinding.executePendingBindings(ViewDataBinding.java:445) at androidx.databinding.ViewDataBinding$OnStartListener.onStart(ViewDataBinding.java:1687) at java.lang.reflect.Method.invoke(Native Method) at androidx.lifecycle.ClassesInfoCache$MethodReference.invokeCallback(ClassesInfoCache.java:216)

hufman commented 3 years ago

Ah ha I've seen that too. By returning an int, Android is trying to load a strings.xml resource by that id. The databinding syntax itself can add a .tostring() in the layout xml, or it could be converted to a double like for the speed, or the LiveData can save the tryAsInt result into a variable and then return it in a wrapping string. The BMW Connected code simply divides the red from the yellow and green categories and returns a boolean that is true for red and false for the rest, which could be a valid approach here too.

Pro-tip: You could temporarily use parking brake or others as a cachedCdsData, run outside to save the cache, and then be able to iterate development faster next to the computer ☺️ Oh wait I see you're doing that already!

drm87 commented 3 years ago

This really looks great. Unfortunately, in my ID4 BMW there seems to be a minor glitch. The fuel range is reported as zero, but in the car data printed below there seems to be the correct value available. I hope the screenshot makes it clear.

3F836CDE-378C-48AC-9C0B-10F4314A9266

hufman commented 3 years ago

Oh very interesting! I just pushed up a change that I need you to try, which adds another field to that advanced info tab. The sensors.fuel range element seems to include both petrol and battery ranges, and so I have the Fuel Level field programmed to take this sensors.fuel range and subtract any EV battery range. In the new build, can you tell me what your car says for driving.displayRangeElectricVehicle? In my tests, it was 4095 for a missing value, and so I currently ignore values at 4095 or higher, but maybe your car has a different value that I should ignore.

drm87 commented 3 years ago

In the new build, can you tell me what your car says for driving.displayRangeElectricVehicle? In my tests, it was 4095 for a missing value, and so I currently ignore values at 4095 or higher, but maybe your car has a different value that I should ignore.

Here it is:

sensors.fuel: {"range":699,"reserve":0,"tanklevel":42}
driving.displayRangeElectricVehicle: 4093
hufman commented 3 years ago

Ok, pushed up that new value, after some ~30 minutes you can download the new build and it should be fixed up. Thanks for testing!

jezikk82 commented 3 years ago

Feel free to test the new version with new real time data. New nightly build will be avaliable tomorrow morning.

Especially owners of the Gxx cars please share feedback about driving modes and parking brake/auto hold (if they are working, are correctly mapped).

unlive74 commented 3 years ago

@hufman

controls.sunroof: {"openPosition":255,"tiltPosition":255,"status":3}

Full list: stuff.txt

unlive74 commented 3 years ago

Is your Last Position field blanked out by you, or just empty due to the car not providing it?

It depends if advanced settings are enabled.

hufman commented 3 years ago

Interesting! So it only shows your GPS coordinates but not a street or city. I'll push up a build with some more Car Data information soon, can you peek at what the navigation.currentPositionDetailedInfo says for you? Your stuff.txt says map:false and navi:false, I suppose this empty street label could happen if the car doesn't have a map internally to name the location. That would explain the empty field, and I suppose it doesn't look that bad... Does the little map pin button show you where your car is correctly?

unlive74 commented 3 years ago

You're right - I don't have navigation in my car, no label navigation.currentPositionDetailedInfo. Gmaps find the car by coordinates.

hufman commented 3 years ago

The new build that just finished should have a fix for the cut-off Compass heading you saw and a new navigation.currentPositionDetailedInfo field. Can you also run the Sentry version to log the crashes you mentioned?

unlive74 commented 3 years ago

Can you also run the Sentry version to log the crashes you mentioned?

I've done it an hour ago (20:18 GMT+5), the application crashed 3 or 4 times ...

navigation.currentPositionDetailedInfo: null

hufman commented 3 years ago

Hmmm I found one client in the analytics that matched your stuff.txt report, and all it reported were successful connections from a Galaxy S8, no crashes were captured at all =\ Try getting an adb logcat dump to see if that has the error.