mathiasvr / bluejay

:bird: Digital ESC firmware for controlling brushless motors in multirotors
GNU General Public License v3.0
487 stars 51 forks source link

Initial attempt in adding docker as a build container. #11

Closed stylesuxx closed 3 years ago

stylesuxx commented 3 years ago

It turns out that working with the Keil toolchain is quite painfull. Each user requires his own license. Although the license being free, we can't just provide a pre-populated tool chain. Developers will need to properly register their Keil toolchain and can then use the docker image for building. Right now the docker image is pretty dumb, since the user has to do a lot of stuff by himself. A build script was added to help building all or single targets via docker.

Work in progress - feedback welcome.

mathiasvr commented 3 years ago

I found it was enough to copy user.reg and system.reg from .wine into the docker one. This is a bit faster and for me gave a smaller .wine dir, but maybe that's because there is some bloat in my local one.

Oh I of course also needed to copy the keil bins.

stylesuxx commented 3 years ago

Makes sense. Will adapt it accordingly, maybe add an empty .wine directory to the repo and instruct the user to only copy those files.

saidinesh5 commented 3 years ago

@stylesuxx Could you also update the Makefile to run things with proper WINE_PREFIX?

mathiasvr commented 3 years ago

We can also have the user copy the whole .wine dir and only copy the needed files to docker. Maybe note that the .wine dir can be removed after docker image has been made.

stylesuxx commented 3 years ago

We can also have the user copy the whole .wine dir and only copy the needed files to docker.

Yes - this I think is the better idea.

Maybe note that the .wine dir can be removed after docker image has been made.

Will add.

mathiasvr commented 3 years ago

@stylesuxx Could you also update the Makefile to run things with proper WINE_PREFIX?

Docker setup works fine with default wineprefix, are you referring to the simplicity studio prefix? I haven't used it because I uninstalled simplicity studio, but maybe we can have some case to support it.

saidinesh5 commented 3 years ago

@mathiasvr I think mixing and matching system wide wine with the tools from simplicity studio gave me weird registration errors when i tried. I assume it is due to computer id changing or something like that. And i had to manually specify the WINE_PREFIX and Simplicity studio's wine to make it work.

Hence my suggestion that if we specify the wine binary in the makefile, specifying the WINE_PREFIX too would make sense for those of us not using docker.

mathiasvr commented 3 years ago

@mathiasvr I think mixing and matching system wide wine with the tools from simplicity studio gave me weird registration errors when i tried. I assume it is due to computer id changing or something like that. And i had to manually specify the WINE_PREFIX and Simplicity studio's wine to make it work.

Hence my suggestion that if we specify the wine binary in the makefile, specifying the WINE_PREFIX too would make sense for those of us not using docker.

I see. I just suggested removing the wine binary from the makefile, but I guess this could be problematic if simplicity uses a different version? Well, maybe we need both wine binary and prefix in the makefile then.

@stylesuxx Do you think you can add a WINE_PREFIX variable to the makefile that is used by the wine command? Sorry about the conflicting proposals.

mathiasvr commented 3 years ago

Wait I just realized WINEPREFIX is an environment variable and not a wine option, then it doesn't really need to be in the makefile to take effect. I think it's fine to default to the default wineprefix and location since we cannot know where simplicity is installed anyway.

mathiasvr commented 3 years ago

Thanks @stylesuxx

I actually like this because even if you don't use docker you can still follow the docs and it should work with local wine right? I mean do we even need simplicity studio at all?

stylesuxx commented 3 years ago

What still needs to be changed in my opinion is KEIL_PATH. This should probably be an env variable - then we can point it to simplicity or the source we use in docker.

WINEPREFIX is an ENV variable anyway - I don't think I need to add it to the Makefile.

saidinesh5 commented 3 years ago

@mathiasvr Can confirm that exporting both WINEPREFIX and PATH such that simplicity studio's wine and it's configuration are found in this Makefile works.

mathiasvr commented 3 years ago

What still needs to be changed in my opinion is KEIL_PATH. This should probably be an env variable - then we can point it to simplicity or the source we use in docker.

It can be overridden with make KEIL_PATH="...". Is that sufficent? I don't mind making it env variable if that's better.

mathiasvr commented 3 years ago

@mathiasvr Can confirm that exporting both WINEPREFIX and PATH such that simplicity studio's wine and it's configuration are found in this Makefile works.

Okay that's good. I think with this approach we don't even need to install simplicity studio at all though. I'm considering even removing it from the docs. I didn't find it especially useful when just writing assembly but maybe I gave up to quick. Do you use/find simplicity studio useful?

stylesuxx commented 3 years ago

It can be overridden with make KEIL_PATH="...". Is that sufficent? I don't mind making it env variable if that's better.

IMHO that is sufficient, yes - since it will then not be used from the build script anyway.

saidinesh5 commented 3 years ago

I didn't find it especially useful when just writing assembly but maybe I gave up to quick. Do you use/find simplicity studio useful?

Oh i never used simplicity studio.. I installed it a while ago when tinkering with BlHeli_S from a youtube tutorial. Have been reusing the same environment with a regular text editor for Bluejay. I tried to replace Keil's tools with sdcc / asem51 but it seemed like more work than i anticipated, so gave up.

mathiasvr commented 3 years ago

Oh i never used simplicity studio.. I installed it a while ago when tinkering with BlHeli_S from a youtube tutorial. Have been reusing the same environment with a regular text editor for Bluejay. I tried to replace Keil's tools with sdcc / asem51 but it seemed like more work than i anticipated, so gave up.

Okay cool, I tried the same thing haha. I don't really think we need to rely on simplicity studio then. @stylesuxx isn't it correct that using your Registering via WINE approach sets everything up just fine even if not using docker?

If so I think we can remove the simplicity studio stuff from the readme since I like this better.

stylesuxx commented 3 years ago

@stylesuxx isn't it correct that using your Registering via WINE approach sets everything up just fine even if not using docker?

That is correct.

If so I think we can remove the simplicity studio stuff from the readme since I like this better.

Done.

As far as I can see, that should be it. If so, let me know and I will squash the commits.

mathiasvr commented 3 years ago

Looks good! 👍