[DONT MERGE] Stable v4.4 vs Stable v4.3 #11276
Conversation
|
Not sure it's such a great idea to back port everything that can be back ported over as opposed to a piecemeal approach for only the most important stuff that would need to come over. The reason being is that this version won't see the light of day as a daily which gets extended use before moving to stable. It could likely be pretty flaky without knowing. |
|
@DonLakeFlyer I agree. However, it's hard to draw the line. If we include some features like gimbal control (which is why we're doing this), then we ought to include other features as well that are well contained. I assume you understand that we're in a bit of a tricky situation trying to release something until all the big/great refactoring (and Qt6) is done and working for each and every config. I assume it's ok to drop v4.4 if v5 can be released within a few weeks but we probably don't want to wait for too long either. |
|
I understand both your points, but I think we are in a particular situation regarding master right now. I am not sure if it is the status right now, but for some time it has been broken, with this process of porting to qt 6, etc. At least in my close contacts, the very few people/companies that used to use always master instead of stable, stopped doing so momentarily because of this instability in master lately. So I think this is important to consider it in the argument of some features going to stable before spending enough time in daily first. On a different note, I recently checked the above commits when rebasing the gimbal PR to 4.4, and I think the highest risk feature of getting through with little testing is probably video tracking, and I don't think it is high risk anyway. The rest of the commits are mainly bug fixes, some of them very important, like that crash fixed in 19462a3, so from my point of view I think it isn't too crazy to include all of this in the next 4.4 release. But of course, if we have a defined timeline for master and it is reasonable, I am okay too with cherry-picking just less stuff, as I see Don's point of the added overhead on including this now. By the way Don, if you could take a quick look at #11264 to be sure you are fine with it, I fixed all the stuff you suggested in the initial draft PR. I was holding on considering it good until you give the ok. Thank you very much! |
|
Slightly tangential and at the risk of overextending myself I have been thinking for a few months that I should perhaps assist somehow with the release management of QGC. Perhaps I could assist with the issue analysis and tracking like I do for AP Copter and Rover. |
|
Now that I'm done with the gstreamer fixes, the cmake stuff is mostly usable for test builds. There are a few more things that would need to be done before a release, but nothing too significant. I had a lot of feature improvements and other stuff I was working on but I can help run through a bunch of fixes first if that would help. I believe many of the issues are due to changes in QML widget properties/signals in Qt6. It's not really my place to comment as just some guy, but we could resolve as many issues as possible for a release and then switch over to CMake afterwards. |
|
Thanks for the offer @rmackay9, I think any help in that direction would be very valuable! @HTRamsey so you're saying we should/could just go straight to v5 based on Qt6 instead? Last time I checked it was missing a few things, and I wasn't sold on all UI changes. That's probably a broader discussion and I need to do some testing before that. |
|
Uhhh personally I think some kind of v4.4 is a good idea because v5 really should be extensively tested for a few months. But in theory, I think most of the general Qt5 -> Qt6 updates are pretty close to 'done' and mostly just need a thorough vetting. Then it just comes down to what features/UI changes do you want included in v5 vs a theoretical v5.1. I guess the point I was making in the previous comment was that making a release based on qmake & Qt6 (for v5.0) is probably a good idea before permanently switching to CMake and potentially introducing any issues from the new build system, unless v5.0 gets put off for at least a few months to allow a lot of testing time. Also I can hold off on any feature changes I had planned and just help fix bugs for either v4.4 or v5.0 if that is the priority. I also think it would be beneficial to make some efforts to improve the auto testing at some point. |
That would be incredible. I have limited hours in a day to work on QGC. If a set of folks could help with issues and then just assign them to me (after they are determined to be real enough and have good details) I can spend my time fixing problems. I can tend to fix things very quickly. Regarding the current state of master: If folks are finding it unstable for general use that should be an easily fixed problem. The majority of changed are UI in nature which tend to take little effort to fix once known. The key is "once known". I took a quick look through new Issues. Other than some some video stuff I'm not seeing much reporting on a pile of problems with master. So that would seem to indicate to me that people are trying daily but not helping out with reporting the problem they find. If people that have contacts can ask them to get back to using daily again on a normal basis and report the problems I will get it fixed up ASAP. It should not take very long at all. As far as creating a new Stable release can someone explain the driving force between needing this? I see a few things:
So for me the question remains, why do we need a new Stable so quickly after the last Stable was released? We have never done that before. Why can't we follow a regular, longer release cycle. |
|
I'm very interested in getting the new gimbal stuff out to the community because it's a great leap forward in terms of usability for one of the major use cases of drones. I also see tons of manufacturers hacking up their own versions of MP and QGC which is a clear sign that this should be in the main version. I actually agree that there hasn't been much bake time for the gimbal stuff so I think it makes a lot of sense to go into Daily, go through some beta testing, there will undoubtedly be some fixes and then finally release it as stable. .. but let's get that darn gimbal PR merged asap! If I were to run a QGC gimbal beta on the AP forums would that be OK? that's where I'm comfortable, have the necessary logins, and it would also allow me to weed out the AP specific issues from the actual QGC issues. I'd also likely get backup from other devs and pro users. |
That would certainly make me feel more comfortable. That said, there are two options here:
The approach for either would be to work from Stable_V4.3 as the base as opposed a true daily build. Folks can install from the CI artifacts for testing. If @rmackay9 is committed to running a beta on this to wring out it. And if @julianoes and @Davidsastresas are committed to responding to issues and fixing problems found in the beta. Then I'm fine with taking approach 2 which is a more comprehensive set of fixes prior to a major 5.0 release. |
|
The way I see "Stable 4.4" is that it is most bugfixes that have been merged since 4.3 as well as some features, such as gimbal, which is lower risk to me as it is a new feature on top of the existing base, rather than "everything new, better and different" that I see in master. My fear is that it will take a long time until we can get a release out with master but I might be wrong with that. So, given that, the reasonable thing to do, is probably try out master and collect the issues that I see, and make a more informed decision after that. |
|
It's absolutely going to be a while before a new stable comes out on master. I'm going through a huge UI rework which is going to take a while to finish and settle out. So my suggestion is if folks are happy with signing up for the work is to create a new stable from this pull and the gimbal pull and then wring it out with a beta test group and call it 4.4 |
|
Ok, so it looks like @julianoes is ok with the path I proposed. Waiting to hear back from @Davidsastresas if he's ok with it as well. After that we can pull the trigger on merging into Stable_4.3 for a new 4.4 release. |
|
Sorry but I got a bit lost on the last 3 messages @DonLakeFlyer. I think you meant if I am okay with doing this over 4.3, dealing with it as a beta and delivering executables in that forum thread managed by @rmackay9 , correct? If so, I am okay with it, and I will be responsive in the following weeks if any problem arises regarding that. Also, just to be sure we are on the same page, you meant also to do the equivalent gimbal PR for master now? or you were implying to give it a bit of time in that beta testing group before preparing a gimbal PR for master? Thanks. |
|
Don's plan sounds OK to me. Re running the release, I'm happy to fit in with however you guys normally run do it but my current thought is to do something like this:
|
Yess, that is correct.
That's up to you. It may be hassle to manage it from both 4.3 and master sides. If so, it's perfectly fine to wait until 4.3 settles out and then carry it across one time to master. @rmackay9 What you listed out looks fantastic. @julianoes Do you think you could notify folks on the PX4 side with a similar but likely lower key beta notice? I assume gimbal UI stuff in QGC is all generic so If we wring it out against ArduPilot then all that carries over toe PX4. So no need to double up testing on both sides. So given all of this I'm going to take one last look through @Davidsastresas pull of gimbal work on 4.3 and merge that in. After that I'll look through this pull again to make sure there isn't anything which looks problematic. Once those two pulls are in we should be able to announce beta. Hopefully, by tomorrow. |
|
Ok, so I've reviewed this pull. It's not a huge set of changes but it's also not a minor change. My suggest to wring this out would be to run a beta on the PX4 side with some folks to do some overall testing on QGC (as opposed to gimbal focused). I also think the person who did the camera tracking stuff should be brought in to retest since that is a major feature which is coming over from master. @julianoes Do you think you could run a beta on the PX4 side for this? |
|
Not sure what happened here. Seems like my review comments are gone now and the whole chain of discussion that was here is gone as well. Is that the case for everyone or just me? |
|
No it's still there |
|
Duh! Anyway, so the Gimbal stuff is in now. So next steps is fix this up to my review comments and merge this in as well. Then we will be ready to announce a beta. |
|
@DonLakeFlyer yes, I'll try to have PX4 people try it out as well. Is there a link, like a pre-release github tag where it can be downloaded from? Regarding your review commits, thanks! I need to find out which PR/commits introduced them and drop them. |
|
@DonLakeFlyer I saw 3 comments by you and dropped the two commits, then force pushed. Alternatively, you can also review which commits went in via the commit list. |
No. Folks will need to install from GitHub CI artifacts. You could also copy those links and put them in a forum post. |
The map scale width changes when the zoom up/down buttons are clicked. When buttons are on the right, and when the user quickly clicks a zoom up/down button several times, the first click change the scale, which changes the buttons position. Subsequent clicks are most of the time not where the user wanted to put them. Putting buttons on the left of the scale will keep them at a fixed position, fixing this unfortunate behavior. Signed-off-by: Julien Olivain <[email protected]>
As requested here: PX4/PX4-user_guide#2072 (comment) Improves readability
That is expected. Android takes the tag to compute the android version, and it needs clean major.minor.patch.dev naming. So, if the tag would be 4.4.0.2 instead Android CI would build fine, it is that "rc2" that confuses android build. By the way, that can be reproduced in local builds too. However, that means it will get built as Stable and not as Daily, as it would have a proper tag associated, and I am not sure if we wanted to do this release candidates as Daily or Stable, so it is something we might want to consider, before changing the tag naming to solve this CI issue. |
|
Oh, right @rmackay9. So we can ignore it, and then make another RC when required. RCs should be Stable, I'd say. |
Otherwise it is not found for Android.
|
The Herelink .apk is available now as well, thanks @julianoes! I've also found this random YouTube video that gives pretty good instructions on how to install it. |
This removes rc version strings from the Android version code which does not allow for anything but numbers. This is just to fix the release candidate builds.
|
Here is a warning to fix: |
The used commands like REQUEST_CAMERA_SETTINGS and REQUEST_CAMERA_INFORMATION are deprecated. Therefore, we should gradually try to move to the new REQUEST_MESSAGE commands. This commit does so by sending REQUEST_MESSAGE by default but falling back to the previous commands on the second try and every other retry after that.
By querying autopilot for the CAMERA_INFORMATION message, we allow an autopilot to be a proxy for a MAVLink or non-MAVLink camera. The idea is similar to a gimbal manager implemented by an autopilot.
|
From a quick test this latest change fixes the recording button! |
Since we no longer request cameras to have specific camera component IDs but allow the autopilot to "be" a camera as well, we need to adjust the unit tests to account for that.
This fixes terrain download on Ubuntu 22.04 where only OpenSSL 3 is available on the system but Qt5 wants OpenSSL 1.x. This just copies the fix over from QGCFileDownload.
This fixes getting the video stream.
|
You may want to consider including #11269 since that is kinda relevant to all of the camera work |
|
Done, thanks @HTRamsey. |
|
Everything looks very nice, thank you everybody. I just pushed a couple of commits fixing the qml warning Julian posted a few comments back, and also fixing the offline maps menu, it was broken after some of the commits cherry-picked from master. |
|
Hi, I think maybe we should consider one more -rc (aka -rc4) and leave it out there testing for a few days and if that goes OK then release it as the official version? The feedback on the beta testing thread has slowed so a new -rc may help get some additional feedback. We have a few issues on the issues list but I think they're not critical. The "Point Home ROI can't be cancelled," might be nice to fix but it's not the end of the world. |
This made sense when we were not taking compid autopilot into account, but now it will log this every vehicle connection if it isn't emulating a mavlink camera
|
Hi Randy. I checked the matter about cancel ROI for home. It can be done, as it is now, you can click on the label instead of the waypoint: If clicking the circle icon, it will take it as waypoint and show the "change waypoint to ... " dialog. But if you click the label "ROI Here", it will prompt you with the options to disable ROI. I think we can do an rc5 with the newer changes and try to get more feedback. Thanks! |
|
I've updated the links to -rc5 on the AP beta testing thread. I'm off on holidays until Wed so my ability to test is very limited. Still, I think we should go ahead and release. I think we've done a good bit of testing and enough time has passed during the beta period that we can be confident that it is stable and better than 4.3. Thanks to @julianoes, @Davidsastresas, @DonLakeFlyer and @HTRamsey for their work on this release and also to those who provided feedback during beta testing including:
Thanks! |
|
Closing this pull request as it was just to track the diff but not to be merged. |
|
Here is the release: Thanks @rmackay9 for coordinating and communicating the RCs, and @Davidsastresas, and @HTRamsey for the help, and everyone else for the testing and notes. |
This is just the diff of the
Stable v4.4branch with backports on top of theStable v4.3branch.It's basically an effort to get a v4.4 out before we go to v5 which includes major refactoring and Qt6.
Things that conflict and didn't make it in, are:
Below is the full list that I went through, formatted to avoid backlinks everywhere.
The full list is taken from:
https://github.com/mavlink/qgroundcontrol/pulls?page=9&q=is%3Apr+is%3Aclosed+merged%3A%3E2023-11-30