mauer / xmidictrl

XMidiCtrl is an X-Plane plugin to use midi controllers within the flight simulator
https://mauer.github.io/xmidictrl/#/
GNU Affero General Public License v3.0
32 stars 10 forks source link

Build refactor #21

Closed mjh65 closed 1 year ago

mjh65 commented 1 year ago

Hi Marco,

I'm sending you this PR so you can take a look and see if you are OK with the direction I am taking. The aim of this PR is to separate the core functions of XMidiCtrl from the X-Plane plugin wrapping so that the XMidiCtrl core library can be re-used in alternative environments.

My medium-term goal is to create a command line tool as an alternative wrapper for the XMidiCtrl core functions. I think this will make it easier to (1) debug any enhancements to XMidiCtrl, (2) test a config file independently from the simulator.

There is some further work(*) I'd like to do to finish what I have done so far, but it would be good to get your feedback at this stage.

I would suggest that you don't take this PR in its current form (I have not given this very much testing, or checked that the build is still OK on Linux and Mac). However, if you think the direction is OK and you would be interested in adopting these enhancements then I will complete the restructure and send you an updated PR once it is fully tidied.

Regards, Michael

(*) further work I have in mind is to (1) finish tidying the interface between the core and the plugin so that the core exports its API through a smaller number of abstract interface classes and not by making all the internal header files visible, (2) move the plugin build commands down from the top-level CMakeLists file to match the core library.

mjh65 commented 1 year ago

I've now completed the additional updates I mentioned in the first pull request. I plan to develop the stdio tool on a new branch, but I may submit some further updates to this PR related to any adjustments of the interface between the core and plugin.

Looking forward to your feedback.

mauer commented 1 year ago

Good morning,

Many thanks for all your work.

I have actually done some work myself to enable the plugin to work outside of X-Plane for debugging and testing reason. It's unfortunately not an easy task, as you need to mock quite a few X-Plane functions. I believe a CLI application will not be sufficient. You need some UI to emulate datarefs in order to trigger outbound messages.

Some other project I'm working on can already be run externally for testing reason. Lately, I haven't had much time to invest in XMidiCtrl, but I would like to continue this year. For example, I would like to extend the profile window to show all mappings and devices.

Please continue your work - it looks good!

Thanks, Marco

mjh65 commented 1 year ago

I have actually done some work myself to enable the plugin to work outside of X-Plane for debugging and testing reason. It's unfortunately not an easy task, as you need to mock quite a few X-Plane functions. I believe a CLI application will not be sufficient. You need some UI to emulate datarefs in order to trigger outbound messages.

Yes, I considered the idea of creating an X-Plane mock wrapper and loading the plugin directly, but it felt like a big task. I agree that for full testing a simple CLI will not be sufficient. Perhaps some kind of scripting engine might work? It's also probably not completely necessary - my motivation for this was mainly to help me understand and debug a profile enhancement I want to make. Creating a wrapper that could be used for regression testing is perhaps a long-term nice idea though?

Please continue your work - it looks good!

I have some time at the moment to work on this so I will continue adding commits to my PR branch and let you know when I think it is ready for merge back into your master. Thanks for confirming you are interested in the changes - maintenance becomes much harder if my fork diverges too much from your master!

I've also started wondering about using the XMidiCtrl engine as an add-on to MSFS. I have the SDK, but haven't really done anything serious with it yet, so I don't know if it is possible. For now I'll just focus on refactoring to achieve an API that works for CLI and X-Plane, and worry about MSFS if/when I take this idea seriously!

Best wishes, Michael

mjh65 commented 1 year ago

Hi Marco,

I am reasonably happy with the state of this PR now and will be using it as the base for the rotary switch enhancement that I want to add.

It would be great if you would consider merging this into your repo! Please let me know if you find any issues.

Thanks, Michael

mjh65 commented 1 year ago

Hi Marco. It looks like you have made your own updates to reach the same goal, so I think this PR is no longer useful?

mauer commented 1 year ago

Hi Michael,

I pushed my changes yesterday (or the day before). I was working for quite some time on some refactoring to allow the use of an external program. My goal is to allow XMidiCtrl to start as a normal desktop application, such as the standalone mode of AviTab. I wanted to get that into MAIN before reviewing your PR. Unfortunately, life has been very busy the last few months and I was unable to finish my work earlier.

Do you have any changes in your pull request which could be useful for the project? Be careful with makes changes, such as using #pragma once. The projects has to cross-compile on three different platforms and what works on lets says Windows, often causes problems on Mac or Linux. It can be a real pain - believe me :-)

I will start working on the desktop application, which will use Dear ImGui, such as the plugin. I could create the framework for that and you could fill it with life, if you want to.

mjh65 commented 1 year ago

Hi Marco,

I think your idea of creating a standalone application (like Avitab) is great. (I have also contributed to Folke's Avitab repo, and the standalone app is really useful for testing some features).

One thing I did in the (now out of date) PR was to restructure the CMake files into more standalone projects. That might still be useful, especially for the Visual Studio generation where it's useful to have each of the different parts in separate vcproj files so that you can select (eg) the standalone app (or the stdio tool) as the debugging target. Let me know if you would like me to 'rebase' that part of the PR on the latest code?

If you have some time to create a framework for the standalone application I would be happy to try and get something working. It would help me get familiar with the code too.

BTW, I have a different view from you about header file inclusion guards. I think that using preprocessor guards based on simple filename is more likely to cause issues due to the greater chance of name clashes when using 3rd party libraries. If using preprocessor guards is required I'd propose using a GUID rather than the file name, or at least some prefix to try to introduce some kind of namespacing, eg _XMIDICTRL_UTILSH. But I really think that '#pragma once' is so much easier, and I don't know of any C++ compilers that supports C++20, but doesn't support '#pragma once'. Indeed the imgui library uses '#pragma once', so I guess none of the compilers that can build XMidiCtrl will have any problem?

Thanks, Michael

mauer commented 1 year ago

Hi Michael,

I'm currently adding GLFW as dependency and already got that working to x-compile on all three platforms. Next step will be to create a simple application which draws a ImGUI menu to emulate the X-Plane plugin menu.

Let me finish that first and then you will be able to restructure the CMake files into several standalone projects. I'm not very familiar with Visual Studio, because I use CLion as development platform. So be my guest to change things in order to make life easier :-)

Alright, you convinced my of using pragma once - feel free to change it and I will accept the pull request.

One note about MSFS2020, because you mentioned it. I was thinking about porting XMidiCtrl to MSFS, but dismissed it. There is a really inexpensive application (~20 Euro) called Axis and Ohs which does support MIDI devices. It's a really nice application and I think there is no need for another plugin.

Have a nice weekend!

Kind regards, Marco