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

Please add Dev branch to keep a clean production version #47

Closed
Peemouse opened this issue Aug 5, 2019 · 12 comments
Closed

Please add Dev branch to keep a clean production version #47

Peemouse opened this issue Aug 5, 2019 · 12 comments

Comments

@Peemouse
Copy link
Contributor

Peemouse commented Aug 5, 2019

Hi Ben,

Hope you're well since Paris event.

May you create a Dev branch on the VESC Tool and VESC FW github repos ?
That will help to keep a clean production version.

With more and more contributors (which is fantastic !) it becomes more and more messy and final users are lost due to constant updates (also bugs leak into production code and with VESC Mobile which eases the FW deployment, bugs land into our skateboards we ride everyday...).

Thanks a lot Sir !

Cheers,

Clément

@vedderb
Copy link
Owner

vedderb commented Feb 1, 2020

Hi Clément,

Sorry for the delay, I have been working 16 hours per day, including the holiday season, since the event.

This issue has been on my mind for a long time. It is the main reason I started working on the backwards compatibility in VESC Tool. I would like to have as many people as possible on the latest firmware, as all versions so far except 3.63 and 3.64 have been regression free.

As the latest version of VESC Tool is 100% backwards compatible I recommend keeping the firmware you have if it works, unless VESC Tool warns about a specific issue with that firmware. For VESC Tool I recommend always using the latest version. To decide if you want to update you can check the firmware changelog from VESC Tool and see if there is anything new that you want to use that isn't in your firmware version.

If you want to update to get the latest features you should do that when you are not in a hurry. I might implement a way to read out the firmware before updating to roll back to the previous version if you really need to, but in general those cases should be very rare as long as third party sw isn't involved.

/Benjamin

@Teslafly
Copy link
Contributor

Teslafly commented Feb 14, 2020

Not to mention having a dev/testing branch is a good idea implemented in many oshw projects

Adding a dev branch would also allow pull requests from multiple people to be merged together, tested, and released in 1 major release. The only time you would commit to master is with a squashed pull of commits from the dev branches when you are doing a stable release.

It makes feature development easier as you can get the latest improvements from others without breaking the stable branch.

Additionally, it would allow beta testers to try out features like mtpa coming down the pipeline on many different hardware versions to find problems before the mass release. maybe a 1 week delay from testing to stable release?

@rpasichnyk
Copy link
Contributor

Another benefit of having dev/testing branch is extra time for 3rd party integrations to add support for latest firmware changes.

@Peemouse
Copy link
Contributor Author

Hi @vedderb ,
Thanks for your reply !
First of all, please acknowledge that I'm absolutely not saying you're doing not enough !
This is the opposite, I'm really impressed by the work you've accomplished so far.

However, I think that this project is huge enough to require a dev branch not only for regressions, but also because it involves so many users, contributors or 3rd party dev that it needs a beta version before release.

as all versions so far except 3.63 and 3.64 have been regression free.

Just one can lead to terrible consequences, that's why it's not too late to consider this issue.

I follow some OS projects really terrified by a single regression that they have alpha/beta/release process.
And we're talking about theater lighting SW. 🙂

Why not with the code that may throwing us down at +50 kph ?

@DerelictRobot
Copy link

I have to echo the other sentiments here. If it's a matter of having man-power to maintain a stable/dev branch, I don't think the additional burden is much and I'm sure there are plenty of developers in the community to help out.

@Peemouse
Copy link
Contributor Author

Peemouse commented Mar 6, 2020

Hi Ben @vedderb ,

Sorry to raise again the attention on this "issue".
But it becomes really serious now.
We got a french rider injured because of a regression in the FW 4.01 causing wobbles at high speed.
He got surprised and couldn't recover it.

I can't be more convinced that it wouldn't have happen if there were a group of experimented beta testers which would have raised the problem before releasing the FW to the rest of the community.

Ok, this is more work, this takes time.
But WE ARE HERE.
Here to help you, to support you.
You have a community of thousands of riders willing to help you.
And today, they probably want that the VESC Project workload goes more into safety rather than into new features (even if there are awesome, as always).

I'm staying available to talk about it, or do whatever you need me to, here, by email or by phone (due to the corona virus, not face to face right now 😄 but why not ? I loved the time when I did live in Västeras)

Take care,

Clément

@DerelictRobot
Copy link

DerelictRobot commented Mar 6, 2020

Let me echo the support that Clément is showing here by also offering my time & experience in testing/QA/QC processes. I have prior experience in managing a number of open source Robotics projects, and would be more than happy to assist in whatever ways are needed.

Thanks again for all of your continued hard work & efforts Benjamin!

Cheers,
Andrew Dresner

@vedderb
Copy link
Owner

vedderb commented Mar 6, 2020

I clearly see the problem, and want to make the VESC as reliable as possible. The difficult part here is that it is not always the big changes that makes things break on some setups, it can be small details.

The problem with the wobble is that I changed the default current controller time constant to a higher value. The reason it was low is that the current controller was not decoupled, and needed to be fast to handle sudden load changes without getting overcurrent faults. The problem is that many setups become unstable if the controller is too aggressive, and I have several users who have to change the time constant manually because of this. With the decoupling and wind up protection added in the new firmware I could lower the default value, which made it work by default for more setups I have here. I tested this on my longboard, scooter and ebike, and it was working well for me. I'm not brave enough to go faster than 25 km/h though, so I never discovered that the slower time constant together with the new windup detection and decoupling would cause resonance on a longboard truck at higher speed. Resonances in mechanical systems are really tricky to deal with, even bridges have collapsed for this very reason.

So it was not the large changes in the update that caused the problem, and the new time constant made the setups I have to test work better, but many users were impacted negatively by this. I feel a lot of responsibility for keeping the VESC as safe as possible, and don't like to see when people hurt themselves unnecessarily. When doing firmware updates I point out that you should test things carefully to make sure that everything works, but not everyone does that. Still, I can only emphasize that you should be careful when updating, and only update if you have a good reason and enough time available. I have changed the old firmware warning in the next version of VESC Tool to reflect this.

That being said, it would be great to have more people help me test new versions before I release them. Due to the sensitivity to small changes and my available time, I don't think having a stable version and a testing version of the firmware is feasible at this point. Even the stable version would have to go through a lot of testing before making even small updates, as is perfectly shown with the time constant example, that is not even a firmware feature, but a settings where only the default value has changed. Never updating and calling it stable it is not a solution either as no firmware works for all users with the default settings. Even the slower time constant made all setups I have access to work better, and again, the new firmware is fine for the users with problems if they just change the default value of the time constant. I can even imagine that changing the default ramping times for remotes will make users hurt themselves if they don't expect that, so this is a really tricky problem to deal with.

One way to go ahead with this is to tag official releases on git, and have the commits and/or branches in between as testing versions. For the testing versions, I can provide test builds of VESC Tool where volunteers can test the new features and give me feedback, and if it works well for all testers and my test setup at home, this becomes the new stable version. We should a list with test procedures that have to be performed and checked before the release, to document and ensure that the tests really have been done. These more formal test procedures can evolve over time to catch even more problems. In addition to just running setups, I will also make a stress test setup at home where I can bombard the VESC will commands over CAN, UART and USB at the same time over night, and make sure that nothing gets stuck.

Do you think that is a good way to go ahead? In that case I will start to write and propose a test procedure list here, and get some feedback on it. Then I will try to make sure that it is followed and documented for releases.

@Peemouse
Copy link
Contributor Author

Peemouse commented Mar 9, 2020

Hi Ben,
Thanks for this detailed answer.

The difficult part here is that it is not always the big changes that makes things break on some setups, it can be small details.

From experience, it's mostly small things that lead to the most severe consequences because we don't affect that much interest. We care a lot more to bigger changes.

I understand the technical point, I've nothing against it, you're surely right.
And as you stated, everything looked fine on your setup. For sure, you can't test all setups in the wild.

This is EXACTLY what beta state is made for.
What you do is alpha state testing.
Everything looks good to you, accroding a test procedure, you switch from Alpha to Beta.

Everything is fine for beta testers according another test procedure (could be the same as yours), the state goes to Release and everybody can enjoy it.

Enhancing your test procedure by testing Alpha version on a bench prevent to leak some serious bugs to beta users.
But still, the bench can't reflect all the variables a setup in the real world will meet.

One way to go ahead with this is to tag official releases on git, and have the commits and/or branches in between as testing versions.

This is a good start IMO. But versioning has to be clean !
You stated in the latest VESC Tool commit :
the firmware must be upgraded even if the version number is the same

How can 2 different FW have a different version number ??
This leads to a 100% sure mess. Speaking from experience.

There are no reasons to arrive at this point if we stick to a right flow.
Github has a nice guide for good practices here :
https://guides.github.com/introduction/flow/

I will start to write and propose a test procedure list here, and get some feedback on it.

I'm really interested and we'll contribute 👍

Cheers,

Clément

@vedderb
Copy link
Owner

vedderb commented Mar 10, 2020

I will get started when I'm back home!

This is a good start IMO. But versioning has to be clean !
You stated in the latest VESC Tool commit :
the firmware must be upgraded even if the version number is the same

This is only something temporary I added to the latest VESC Tool, as I sent it to a few people who had wobble problems for testing this release. Wanted to get a stable version out before my trip to California, where I am now for this week. I will add a tag to the firmware and/or a beta build number so that any version of VESC Tool can tell if it is a beta firmware.

@Peemouse
Copy link
Contributor Author

I just saw you created dev branches.
Thank you very much Ben for having taken this into account.
Really appreciate it ! 🙏

@vedderb
Copy link
Owner

vedderb commented Mar 17, 2020

Glad you like it!

I also created a development and test section in my forums, where I will ask for help with testing:
https://vesc-project.com/forum/15

The firmware now has a test build flag as well as VESC Tool to not mix test releases up.

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

No branches or pull requests

5 participants