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

[Proposal] Create a new branch for AirSim compatibility #14615

Closed
bys1123 opened this issue Apr 7, 2020 · 23 comments
Closed

[Proposal] Create a new branch for AirSim compatibility #14615

bys1123 opened this issue Apr 7, 2020 · 23 comments
Labels

Comments

@bys1123
Copy link
Contributor

bys1123 commented Apr 7, 2020

Describe problem solved by the proposed feature

  1. The PX4 + AirSim compatibility usually be broken, PX4 is updating so fast, and AirSim can catch it up. So users cannot run it properly. Maybe they running an ancient version with poor features, maybe they just abandon it.

  2. Why users need PX4 + AirSim?
    a. For an user community it's very useful, lots of beginners wants to run HITL first, to avoid unnecessary crash and experience recent features (old version is useless for beginners).
    b. researchers need AirSim to run drone vision/lidar simulation
    Gazebo world is too rough not enough for modern vision simulation.
    c. AirSim need large number users to get feedback and budget. (not confirmed, just usual practice)

  3. The latest broken was caused by this PR:
    simulator: set sensor update rate according to HIL_SENSOR bitmask #14228 (source: @MaEtUgR )
    Feature: add multi sensor subscription PX4-SITL_gazebo-classic#411 (source: @Jaeyoung-Lim )

  4. There is lots of vision drone competitions recently, they are highly interested in AirSim + PX4 duo, it's a good opportunity to put lots of things together. Like promote PX4 and Pixhawk to researchers, at least avoid confusion Pixhawk hardware standards in their theses.

Describe your preferred solution

  1. A low activity branch can be much easier to maintain, do not need to run AirSim CI on every update.
    I suppose bimonthly to half year maintain is enough.

  2. It's better than a tag could easier to include particular features for AirSim. like this:
    Add SITL target for starting airsim #14497

Describe possible alternatives

At least create a tag on git for AirSim.
Or I'll create a fork to do it.

Additional context

I'm glad to maintain this branch, If it possible, I almost know everybody need and lots of tech details on it.
I can also share recent broken details with AirSim team too, such as test PX4 master weekly, if not compatible at some point, I will let them know, and work out a way with them to fix.

@TSC21
Copy link
Member

TSC21 commented Apr 7, 2020

I don't see where creating a branch specific to Airsim is a solution. The PX4 ecosystem and its tools that are under the maintaince of the core team and being supported by different developers are kept updated whenever possible. Airsim is not under our maintainance, so its the responsibility of who maintains the Airsim repo and code to keep it up to date to the different changes happening upstream and not the other way around.

So my advise is that if one wants to use Airsim and its using it frequently, then that person or group of people should be responsible for bringing the required updates to Airsim, to make it compatible with the latest changes happening in PX4 - and of course considering that one will be want to test or develop over the latest master, otherwise we always have the stable releases of PX4 to which at least Airsim should be synced to work with. Otherwise, the repo maintainers are not doing their job properly.

@bys1123
Copy link
Contributor Author

bys1123 commented Apr 7, 2020

Ok, thanks I'll create a fork then.

BTW, I didn't see AirSim maintainers as bad as you sad. Especially ArduPilot can run with AirSim properly now, this is a big thing, as PX4 is a nice vision drone flight stack in past years.
Which means you can't wait other nice AirSim replacement show up recently, but ArduPilot is replacing PX4 on this area.

cc' @LorenzMeier @mrpollo

@bys1123 bys1123 closed this as completed Apr 7, 2020
@TSC21
Copy link
Member

TSC21 commented Apr 8, 2020

@bys1123 I didn't said the maintainers are bad. You misread what I said.
Instead of creating a fork, why don't you propose a PR to the Airsim repo that makes it compatible with the current state of PX4?

@TSC21
Copy link
Member

TSC21 commented Apr 8, 2020

Also, I don't think this should be a topic to pick up and compare PX4 and Ardupilot. But I wouldn't also say that one is replacing the other just because Airsim is not being kept up to date with upstream PX4.

@hamishwillee
Copy link
Contributor

Indeed, on that basis you could argue no one should improve their simulations (with lockstep etc) or core software product because that might break an external simulator.
But as discussed elsewhere, we do need to have CI support for this so that everyone knows when it happens, not long after the fact.

@Jaeyoung-Lim
Copy link
Member

@hamishwillee @TSC21 Just a FYI, I have reported to the airsim repo that it is currently not compatible: microsoft/AirSim#2477

@bys1123
Copy link
Contributor Author

bys1123 commented Apr 8, 2020

@Jaeyoung-Lim Thanks, this is nice.

@bys1123
Copy link
Contributor Author

bys1123 commented Apr 8, 2020

image
If don't notice them how they would know to they should change their code to keep up to date.

@bys1123
Copy link
Contributor Author

bys1123 commented Apr 8, 2020

@hamishwillee CI is a great stuff, but for heavy project like AirSim, is that fit PX4 CI resources?

@TSC21
Copy link
Member

TSC21 commented Apr 8, 2020

image
If don't notice them how they would know to they should change their code to keep up to date.

Not at all. Airsim should be made compatible with PX4. Not PX4 should be made compatible with Airsim. And if there aren't tests running under Airsim CI to keep the compatibility, it is not the responsibility of PX4. We keep maintained and synced the tools that are under our ecosystem. That's almost as saying that we need to warn everyone that uses PX4 on their own made tools that we did changes and they need to update their code. And we actually do that: we have stable releases and release notes that point the changes made.

@bys1123
Copy link
Contributor Author

bys1123 commented Apr 8, 2020

Not to warn everyone, as Microsoft is Gold member of Dronecode, is that the way you treat your partners?
Did anyone in PX4 core team suggest them they should test PX4 master under Airsim CI? Do you think AirSim guys has mind reading skills?
Did PX4 notice your users which is the latest version works with AirSim?

Also, I don't think this should be a topic to pick up and compare PX4 and Ardupilot. But I wouldn't also say that one is replacing the other just because Airsim is not being kept up to date with upstream PX4.

I'm not saying ardupilot is replacing PX4 in AirSim ecosystem, just very clear to see users is switching to ArduPilot, it's all about PX4 user viscosity.

It's not the time we blame the others. I totally understand it's not PX4 developers maintain job, but at least, communication.

Just suggesting, guys you can do a very small job, to keep users in PX4 community.

@TSC21
Copy link
Member

TSC21 commented Apr 8, 2020

Not to warn everyone, as Microsoft is Gold member of Dronecode, is that the way you treat your partners?

@bys1123 you are completely messing subjects here. Anyway this isn't a discussion to have here and I already spoke with @jinger26 about this subject and there will be a follow up.

Did anyone in PX4 core team suggest them they should test PX4 master under Airsim CI? Do you think AirSim guys has mind reading skills?

It's not about mind reading skills or whatever you call it. It's about open-source projects and integration principles.

Did PX4 notice your users which is the latest version works with AirSim?

No because that's not our role. We don't maintain AirSim under our ecosystem, so we are not supposed to know which version is actually working.

@bys1123
Copy link
Contributor Author

bys1123 commented Apr 8, 2020

I‘m not talking about running CI and full stack maintaining job PX4 should do.
Just tell users and AirSim team which version is latest supported, and what will you do something break compatibility, that is the way to keep compatibility.

CI is powerful, but not magic.

@TSC21
Copy link
Member

TSC21 commented Apr 8, 2020

I‘m not talking about running CI and full stack maintaining job PX4 should do.
Just tell users and AirSim team which version is latest supported, and what will you do something break compatibility, that is the way to keep compatibility.

CI is powerful, but not magic.

CI is enough if well implemented. And it's an automatic process. If it fails, then something is wrong ;)

@bys1123
Copy link
Contributor Author

bys1123 commented Apr 8, 2020

It's not about mind reading skills or whatever you call it. It's about open-source projects and integration principles.

Principles is a dead thing, users is living. As AirSim team will continue doing things in their way, you should do something for your users in minimal support.

@LorenzMeier
Copy link
Member

@bys1123 Hey, really sorry this discussion developed this way. This is not how PX4 is treating it's adopters and partners. I will circle back with @Jaeyoung-Lim to see how we can improve the airsim maintenance situation and will get this fixed. It would be awesome if you could help maintain it.

I will re-open this issue until the core issue you raised is addressed.

@LorenzMeier LorenzMeier reopened this Apr 8, 2020
@TSC21
Copy link
Member

TSC21 commented Apr 8, 2020

@bys1123 I am personally fixing the issue on AirSim. Sorry if anything I said lead you to understand we didn't want to help. I will ping you in the AirSim PR as soon as I raise it. Thanks for raising the issue and offer your help!

@TSC21
Copy link
Member

TSC21 commented Apr 8, 2020

@bys1123 fix in microsoft/AirSim#2549.

@bys1123
Copy link
Contributor Author

bys1123 commented Apr 8, 2020

@LorenzMeier @TSC21 Thanks for explains and all your works, really glad we go to a better track with
AirSim compatibility. I'm glad to keep testing the compatibility and offer my help.

@julianoes
Copy link
Contributor

Is there a way to have AirSim running in CI? If so we should try to make that happen. We could potentially have a MAVSDK test with it.

@mrpollo
Copy link
Contributor

mrpollo commented Apr 8, 2020

Great idea @julianoes perhaps Microsoft can help setup us setup AirSim in Azure

@hamishwillee
Copy link
Contributor

@julianoes @mrpollo It's not exactly a new idea - #14615 (comment)

How can we move past "perhaps someone should do something" to someone doing something? Ramon, you going to take this up with Microsoft?

@Jaeyoung-Lim
Copy link
Member

I believe airsim is now compatible with airsim now as per: https://youtu.be/5YaQAA_nn4g

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants