DroidPlanner / Tower

Ground Control Station for Android Devices
https://play.google.com/store/apps/details?id=org.droidplanner.android
Other
619 stars 554 forks source link

Add BaiduMap v4.0.0 support #1778

Closed droneboost closed 8 years ago

droneboost commented 8 years ago

Hi @ne0fhyk ,

Would you please do code review again?

Linjieqiang commented 8 years ago

Thanks @droneboost .But I have to say,the Baidu offine map is neverless.The people would like to offine map with google map.I implement in my Tower version name "飞鱼地面站",You can google it and get it.Also,I implement the Gaode Map in it.Anyway,I just want to say the Baidu offine map is neverless.Hope you can remove the unless code about the Baidu offine map.Finally,thanks for @ne0fhyk your great work.

droneboost commented 8 years ago

Thanks for comment, @Linjieqiang.

As for "Baidu offine map is neverless", could you please clarify your reason?

"The people would like to offine map with google map." is not enough for me.

onceme commented 8 years ago

@droneboost @Linjieqiang

More options are always good for users with different requirements. So no matter Baidu, Gaode in China or Google, Bing even more maps online/offline from various countries, people would find their way to use them somehow. If you could, please contribute your efforts to upstream. Great thanks for that!

Linjieqiang commented 8 years ago

@droneboost Because long time ago,I implemented Gaode 2D offline map in my "飞鱼地面站".But more people who love drones telled me it is neverless.They want to offine google map only.You can join in my QQ group and ask them or discuss this problem.The QQ group number is "247233288".Thanks.

m4gr3d commented 8 years ago

@droneboost Great job with the pull request! Looking forward to the updates. 👍

m4gr3d commented 8 years ago

@Linjieqiang I agree with @onceme, more options are good for the user since everyone has different requirements. And as @onceme mentioned, contributions to upstream are highly encouraged, and appreciated! This project only gets better thanks to contributions :)

Linjieqiang commented 8 years ago

OK.If having free time ,I will take a pr to this.Because I add more featrues in Tower to suit for Chinese user.

m4gr3d commented 8 years ago

@Linjieqiang sounds great, looking forward to it!

droneboost commented 8 years ago

@ne0fhyk,

Thanks for code review.

I've updated according to your review comments, would you please take a look?

m4gr3d commented 8 years ago

@droneboost I will take a look right away. Regarding the Baidu map key, do I need to generate another key for the app for when it's released to production?

droneboost commented 8 years ago

@ne0fhyk,

do I need to generate another key for the app for when it's released to production?

Yes, You have to register as a BaiduMap developer and apply an API key from their site. http://lbsyun.baidu.com/index.php?title=androidsdk/guide/key

Sounds a little bit trivial, but I would like to help when you need.

m4gr3d commented 8 years ago

@droneboost thanks for the registration info. I'm done with the second pass code review, so let me know once you address the comments, and I'll take another look at it :)

droneboost commented 8 years ago

@ne0fhyk

Thanks for your careful code review. I'll try to fix them soon.

droneboost commented 8 years ago

@ne0fhyk

I've done code fix(except the Collections.emptyList()), would you please do third round code review?

m4gr3d commented 8 years ago

@droneboost great update! Only a few small issues remaining before it's ready for merging. By the way, can you not squash your commits when you push them for the PR. It's easier to see the difference between each commits, as I can just review these differences and not the whole PR each time.

droneboost commented 8 years ago

@ne0fhyk ,

Thanks for code review.

By the way, can you not squash your commits when you push them for the PR

Do you mean not to use "git commit --amend" ?

m4gr3d commented 8 years ago

@droneboost I usually do a regular git commit, followed by a git push in order to push my updates to the repo. Is that what you're doing as well?

m4gr3d commented 8 years ago

@droneboost the reason I ask is because you made several updates to the PR, but only 1 commit is listed.

Thanks!

droneboost commented 8 years ago

@ne0fhyk

I'd like to keep all baidumap related modifications in a single commit history when finally merged to upstream, so I did "git commit --amend --no-edit" + "git push" in each code review fix, then issue pull request from my branch to DroidPlanner:develop branch, that's why you see the same PR in each code review.

If it is inconvenient for you to do code review in such way, I can remove "--amend" in next PR. but when code review is finally done, and you decide to merge baidumap to DroidPlanner:develop, please inform me to issue a new PR with all modification in one commit history, then you just approve the PR. Is it OK?

m4gr3d commented 8 years ago

@droneboost Yes, that sounds good!

droneboost commented 8 years ago

@ne0fhyk , PRed again.

m4gr3d commented 8 years ago

@droneboost looks good to merge! Let's me know once you're done with squashing the commits as we discussed, and I'll merge the PR. Great job!

droneboost commented 8 years ago

@ne0fhyk

I've created the new PR with squashed commits, please check https://github.com/DroidPlanner/Tower/pull/1788

Thanks.

m4gr3d commented 8 years ago

Merged in pr #1788