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

Update SendPOI to include display attributes #103

Merged
merged 5 commits into from
Oct 29, 2019

Conversation

rikroe
Copy link
Member

@rikroe rikroe commented Apr 25, 2019

This PR contains some small changes I made when I tried to send not GPS coordinates but failed. However with this changes you can now not only send coordinates but also display information (such as address, city, etc.) like the original app does:

$ bimmerconnected sendpoi -h
usage: bimmerconnected sendpoi [-h] [--name [NAME]] [--street [STREET]] [--city [CITY]]
                      [--postalcode [POSTALCODE]] [--country [COUNTRY]]
                      username password {north_america,china,rest_of_world}
                      vin latitude longitude

send a point of interest to the vehicle

positional arguments:
  username              Connected Drive user name
  password              Connected Drive password
  {north_america,china,rest_of_world}
                        Region of the Connected Drive account
  vin                   vehicle identification number
  latitude              latitude of the POI
  longitude             longitude of the POI

optional arguments:
  -h, --help            show this help message and exit
  --name [NAME]         (optional, display only) Name of the POI
  --street [STREET]     (optional, display only) Street & House No. of the POI
  --city [CITY]         (optional, display only) City of the POI
  --postalcode [POSTALCODE]
                        (optional, display only) Postal code of the POI
  --country [COUNTRY]   (optional, display only) Country of the POI

Overall superb work you're doing! Your implementation works for me on a 3 series F31 with NBT.

I will create a new issue regarding the address only topic with more information, as I'd like your input on some things.

@robthebold
Copy link
Contributor

I tested this with my '17 i3 + Rex. This PR improves on PR#98 by allowing the name parameter to be properly transmitted with the coordinates message. IMO, these PRs should be committed to the master branch -- is there a maintainer in the house who can approve these or at least just take a look at the changes?

@gerard33 gerard33 requested review from m1n3rva and removed request for m1n3rva August 21, 2019 19:46
@gerard33 gerard33 self-assigned this Aug 21, 2019
@robthebold
Copy link
Contributor

I've been testing this code a little more. When you fill in all the required and optional arguments (lat,lon,name,street,city,postalcode,country) you will get the message delivered to the car with everything but the country populated -- there's one line marked "null".

Here's what it looks like on the display:
20190905_091345

And when you open the message:
20190905_091415

(This will take you to an art museum in Davenport, IA. Well worth a stop if you're traveling across the US on I-80. Surprisingly extensive collection of Haitian art in addition to the American and Regionalist work you might expect. But I digress. . .)

So I had set the country to "USA" but it's missing in the message and something appears as "null". Clearly something is still not quite working. When I just leave out the country name parameter out (since it's optional) the "null" still appears. So it's not just that "country" is being transmitted wrong.

Even if you leave out everything but the lat and lon (which will then populate the message with the gps coords translated into deg,min,sec in place of the dest name) you still see the "null"

However, the "null" bug notwithstanding, the function is usable.

All the caveats about sending a POI as coords apply: you're not going to a street address, you're going to a point and e.g. that point could be inside a building and your nav may not take you to the front door. Or you could end up somewhere around the block from where you want to be behind a fence from your destination. If you can't be totally sure you're setting the coords properly, it might be safer to search for a POI in the in-car nav near your point.

@rikroe rikroe assigned rikroe and unassigned rikroe Oct 17, 2019
@rikroe rikroe requested a review from gerard33 October 17, 2019 19:02
Copy link
Member

@gerard33 gerard33 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

@gerard33 gerard33 merged commit 736e38c into bimmerconnected:sendpoi Oct 29, 2019
gerard33 added a commit that referenced this pull request Oct 29, 2019
* tried to imlement "send poi" feature, but it's not really working...

* Uploading the POI to the vehicle seems to be working. Needs to be checked in the car.

#66

* Update SendPOI to include display attributes (#103)

* Update SendPOI to include display attributes

* Fix null in car message, strongly typed parameters in PointOfInterest

* Add basic tests for PointOfInterest

* Fix flake8 & pylint errors

* Update vehicle.py
@rikroe rikroe mentioned this pull request Oct 30, 2019
@rikroe rikroe deleted the sendpoi branch March 28, 2020 12:13
@lock
Copy link

lock bot commented May 5, 2020

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked and limited conversation to collaborators May 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants