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

Custom devices new #258

Merged
merged 11 commits into from
Oct 23, 2023
10 changes: 8 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@ jobs:
python -m pip install --upgrade pip
pip install --upgrade platformio

- name: Checkout custom devices
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree... we should not do this and use the platform.io feature for including additional dependencies

uses: actions/checkout@v3
with:
repository: elral/MobiFlight-CustomDevices
path: CustomDevices

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we'll never have custom devices in default MF firmware should we have this commented out in the repo? Then tell people building firmware with a custom device where to go to uncomment it for their own builds?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why should we not provide "some" custom devices which are officially reviewed and released?
And how do get users a firmware file if they uncomment this with the correct FW Version? I am doing it always by changing the get_version.py script ;)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One question about this. The script has Platform.IO run to to build the firmware. Doesn't Platform.IO itself clone the CustomDevices repo? What's the reason why this also has to be done in the script? Asking because I'm not that experienced in this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Until we have actual custom devices that ship with Mobiflight firmware directly we shouldn't be running a pull for them in the build.

When the day comes that we do decide to ship custom devices with the core firmware that PR can enable the pulling of the custom devices repo.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohhh never mind, I see there's custom devices coming already just the repo needs to be moved over. Right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One question about this. The script has Platform.IO run to to build the firmware. Doesn't Platform.IO itself clone the CustomDevices repo? What's the reason why this also has to be done in the script? Asking because I'm not that experienced in this.

Difficult for me to explain.
If you have a plain Installation, the folder /CustomDevice does not exist. Therefore the files listed in platformio.ini are not found and these environments get not into the list of environments to be compiled. On the next executing of the platformio.ini these files are available on they are getting into the list of environments to be compiled. As the automatic build process has always not the CustomDevice folder and pio run is only executed once, the custom device repo has to be cloned before.
Or what just comes in mind, a pio run - e mega and next a pio run could be done

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't remember if I asked this yet, but are these custom devices from the repo that has to move over intended to ship as part of the core Mobiflight firmware? They're just added using the custom device method instead of being coded as new specific devices?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are fully functional devices. From my side all can be moved to the new Mobiflight repo. And from my side all could be shipped with the main firmware.
But in the end it is the decision of @DocMoebiuz which one will be moved/shipped.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DocMoebiuz If the intention is to ship these as part of the core firmware they should be checked in to the firmware repo, not some external repo and then built in. If the intention isn't to ship them then these build steps should be disabled in the official firmware repo with guidance given to 3rd parties about how to enable them for their own builds.

Aside from complicating the build and local development process we lose valuable git benefits by storing them somewhere else:

  1. There's no easy way to know which iteration of the devices were built into a version of the firmware. The commit hash won't tell us and the source won't be included in the .zip attached to each release.
  2. Any changes made to those devices won't be included in the automatically generated release notes. We'll need to remember to manually add them every time.
  3. It'll be yet another repo for us to have issue tracking. We'll have to remember to look over there for issues related to code that directly ships in the firmware.

Copy link
Collaborator

@DocMoebiuz DocMoebiuz Oct 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DocMoebiuz If the intention is to ship these as part of the core firmware they should be checked in to the firmware repo, not some external repo and then built in. If the intention isn't to ship them then these build steps should be disabled in the official firmware repo with guidance given to 3rd parties about how to enable them for their own builds.

Aside from complicating the build and local development process we lose valuable git benefits by storing them somewhere else:

  1. There's no easy way to know which iteration of the devices were built into a version of the firmware. The commit hash won't tell us and the source won't be included in the .zip attached to each release.
  2. Any changes made to those devices won't be included in the automatically generated release notes. We'll need to remember to manually add them every time.
  3. It'll be yet another repo for us to have issue tracking. We'll have to remember to look over there for issues related to code that directly ships in the firmware.

@neilenns thanks for your valuable feedback! In general, I want to create an opportunity for the community to extend MobiFlight at their on pace and without having me (or us) become the bottleneck. I don't want to be part of the custom device life cycle. At the moment I don't see any custom device become built-in in the official mobiflight firmware. I envision and hope that people come up with their custom device implementations and at the moment I expect them to compile a specific firmware versions, hence board type, for it - or leave it up to the individual user to compile a custom firmware on their own.

@elral has experimented with including different custom devices as repository references. It reminds me of npm or pip where you install a library. There are still some lines of code that have to be adapted to use those custom devices and expose them correctly. But i think for the first release that is perfectly fine and makes it very easy to get started. our assumption is that a user who develops custom firmware will find the process easy enough.

So to specifically answer your questions:

  1. There's no easy way to know which iteration of the devices were built into a version of the firmware. The commit hash won't tell us and the source won't be included in the .zip attached to each release.

... that's OK. the custom firmware can have it's own version. It will have a base firmware version of the official mobiflight firmware and a combination of custom device versions... but it is the responsiblity of the custom firmware creator to maintain this up-to-date in case there are improved versions available.

  1. Any changes made to those devices won't be included in the automatically generated release notes. We'll need to remember to manually add them every time.

... that's OK, I see these devices outside of the official releases anyways. If we have an official MobiFlight Community Repository with curated custom devices (like npm registry) then we can have release notes in that repository.

  1. It'll be yet another repo for us to have issue tracking. We'll have to remember to look over there for issues related to code that directly ships in the firmware.

... that's OK, this is the responsibility of the respective device developer or repository maintainer. For the beforementioned official MobiFlight Custom Device repository I am sure that we can find a feasible solution.

And to say it one more time: I see a big advantage for the community to decouple the generic devices from the official MobiFlight Core Firmware. We keep it this way on purpose so that the community can move on its own pace. We can still think of some incubator process later which might help to bring popular and stable custom devices to the official MobiFlight Community Repository.

Maybe we can find a cool way to automatically create custom firmwares in the future, where the user has the ability to select from a list of available devices the ones they like to have in their firmware. I am convinced that this first iteration provides already enough options to unlock a lot of creativity.

- name: Run PlatformIO
env:
VERSION: "0.0.${{ github.event.number }}"
Expand All @@ -43,5 +49,5 @@ jobs:
with:
name: firmware
path: |
.pio/build/**/mobiflight*.hex
.pio/build/**/mobiflight*.uf2
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved to the environment within platformio.ini to differ between plane mobilflight firmwares and supporter/user firmwares in the name

.pio/build/**/*.hex
.pio/build/**/*.uf2
14 changes: 10 additions & 4 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,12 @@ jobs:
python -m pip install --upgrade pip
pip install --upgrade platformio

- name: Checkout custom devices
uses: actions/checkout@v3
with:
repository: elral/MobiFlight-CustomDevices
path: CustomDevices

- name: Extract build version
id: get_version
uses: battila7/get-version-action@v2
Expand All @@ -46,14 +52,14 @@ jobs:
with:
name: firmware
path: |
.pio/build/**/mobiflight*.hex
.pio/build/**/mobiflight*.uf2
.pio/build/**/*.hex
.pio/build/**/*.uf2

- name: Release
uses: softprops/action-gh-release@v1
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
with:
files: |
.pio/build/**/mobiflight*.hex
.pio/build/**/mobiflight*.uf2
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copy all .hex and .uf2 files to the zip folder, not only these with mobiflight*

.pio/build/**/*.hex
.pio/build/**/*.uf2
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@
.vscode/launch.json
.vscode/ipch
.vscode/arduino.json
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the folder CustomDevice gets cloned during building. To have no changes shown this folder is excluded from synchronization

.CustomDevices
6 changes: 6 additions & 0 deletions _Boards/Atmel/Board_Mega/MFBoards.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@
#define MF_MUX_SUPPORT 1
#define MF_DIGIN_MUX_SUPPORT 1
#endif
#ifndef MF_CUSTOMDEVICE_SUPPORT
#define MF_CUSTOMDEVICE_SUPPORT 1
#endif

#ifndef MAX_OUTPUTS
#define MAX_OUTPUTS 40
Expand Down Expand Up @@ -69,6 +72,9 @@
#ifndef MAX_DIGIN_MUX
#define MAX_DIGIN_MUX 4
#endif
#ifndef MAX_CUSTOM_DEVICES
#define MAX_CUSTOM_DEVICES 5
Copy link
Contributor

@neilenns neilenns Sep 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We screwed up when adding shifters because we did output first and just called them "shifter". Then when I came and added input shifters they got called "inputshifter".

Let's not make the same mistake again with custom devices and get it right the first time. Call them CUSTOM_OUTPUT_DEVICES and make sure all the relevant files are called "CustomOutputDevice".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I second that, good point.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needed some time to think about.

I am still not convinced that it's required to rename it (in contrast to the shifters). The user can setup a custom device as input device, output device or both (how to name in this case). In the end it is a custom devices and the code defines what it is. From the firmware side mainly only a handler is missing which gets called if a defined action is ececuted. For the connector it's function (input and/or output) could be marked in the accordingly device.json file.

#endif

#ifndef MOBIFLIGHT_TYPE
#define MOBIFLIGHT_TYPE "MobiFlight Mega"
Expand Down
10 changes: 6 additions & 4 deletions _Boards/Atmel/Board_Nano/MFBoards.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@
#define MF_MUX_SUPPORT 1
#define MF_DIGIN_MUX_SUPPORT 1
#endif
#ifndef MF_CUSTOMDEVICE_SUPPORT
#define MF_CUSTOMDEVICE_SUPPORT 2
#endif

#ifndef MAX_OUTPUTS
#define MAX_OUTPUTS 18
Expand Down Expand Up @@ -69,10 +72,9 @@
#ifndef MAX_DIGIN_MUX
#define MAX_DIGIN_MUX 3
#endif

#define STEPS 64
#define STEPPER_SPEED 400 // 300 already worked, 467, too?
#define STEPPER_ACCEL 800
#ifndef MAX_CUSTOM_DEVICES
#define MAX_CUSTOM_DEVICES 2
#endif

#ifndef MOBIFLIGHT_TYPE
#define MOBIFLIGHT_TYPE "MobiFlight Nano"
Expand Down
6 changes: 6 additions & 0 deletions _Boards/Atmel/Board_ProMicro/MFBoards.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@
#define MF_MUX_SUPPORT 1
#define MF_DIGIN_MUX_SUPPORT 1
#endif
#ifndef MF_CUSTOMDEVICE_SUPPORT
#define MF_CUSTOMDEVICE_SUPPORT 2
#endif

#ifndef MAX_OUTPUTS
#define MAX_OUTPUTS 18
Expand Down Expand Up @@ -69,6 +72,9 @@
#ifndef MAX_DIGIN_MUX
#define MAX_DIGIN_MUX 3
#endif
#ifndef MAX_CUSTOM_DEVICES
#define MAX_CUSTOM_DEVICES 2
#endif

#ifndef MOBIFLIGHT_TYPE
#define MOBIFLIGHT_TYPE "MobiFlight Micro"
Expand Down
6 changes: 6 additions & 0 deletions _Boards/Atmel/Board_Uno/MFBoards.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@
#define MF_MUX_SUPPORT 1
#define MF_DIGIN_MUX_SUPPORT 1
#endif
#ifndef MF_CUSTOMDEVICE_SUPPORT
#define MF_CUSTOMDEVICE_SUPPORT 2
#endif

#ifndef MAX_OUTPUTS
#define MAX_OUTPUTS 18
Expand Down Expand Up @@ -69,6 +72,9 @@
#ifndef MAX_DIGIN_MUX
#define MAX_DIGIN_MUX 3
#endif
#ifndef MAX_CUSTOM_DEVICES
#define MAX_CUSTOM_DEVICES 2
#endif

#ifndef MOBIFLIGHT_TYPE
#define MOBIFLIGHT_TYPE "MobiFlight Uno"
Expand Down
7 changes: 3 additions & 4 deletions _Boards/RaspberryPi/Pico/MFBoards.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,9 @@
#ifndef MAX_DIGIN_MUX
#define MAX_DIGIN_MUX 4
#endif

#define STEPS 64
#define STEPPER_SPEED 400 // 300 already worked, 467, too?
#define STEPPER_ACCEL 800
#ifndef MAX_CUSTOM_DEVICES
#define MAX_CUSTOM_DEVICES 5
#endif

#ifndef MOBIFLIGHT_TYPE
#define MOBIFLIGHT_TYPE "MobiFlight RaspiPico"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For all boards the custom devices is added, but excluded in build process for the existing environments

Expand Down
12 changes: 12 additions & 0 deletions get_CustomDevices.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import os

Import("env")

CUSTOMDEVICES_DIR = env.subst("$PROJECT_DIR/CustomDevices")

if not os.path.exists(CUSTOMDEVICES_DIR):
print ("Cloning Mobiflight-CustomDevices repo ... ")
env.Execute("git clone --depth 100 https://github.com/elral/MobiFlight-CustomDevices $PROJECT_DIR/CustomDevices")
else:
print ("Checking for Mobiflight-CustomDevices repo updates ... ")
env.Execute("git --work-tree=$PROJECT_DIR/CustomDevices --git-dir=$PROJECT_DIR/CustomDevices/.git pull origin main --depth 100")
2 changes: 1 addition & 1 deletion get_version.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,4 @@
])

# Set the output filename to the name of the board and the version
env.Replace(PROGNAME=f'mobiflight_{env["PIOENV"]}_{firmware_version.replace(".", "_")}')
env.Replace(PROGNAME=f'{env["PIOENV"]}_{firmware_version.replace(".", "_")}')
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As now firmware names should start with the supporter/user name, mobiflight is moved to the environments

47 changes: 30 additions & 17 deletions platformio.ini
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,14 @@
; development use VSCode to change the target to a non-default
; by clicking on the target name in the bottom status bar.
[platformio]
; include custom environments to be build
extra_configs =
./CustomDevices/KAV_Simulation/EFIS_FCU/EFIS_FCU_platformio.ini
./CustomDevices/Mobiflight/GNC255/GNC255_platformio.ini
./CustomDevices/Mobiflight/TM1637/TM1637_platformio.ini
./CustomDevices/Mobiflight/GenericI2C/GenericI2C_platformio.ini
./CustomDevices/_all_CustomDevices/all_devices_platformio.ini


Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Include builts from external files which are part of the custom device repo and cloned.
For new custom devices onyl one entry has to be added. No more changes on this repo are necessary.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are any of these actually going to ship with the main mobiflight firmware? If so, where is the build step that clones them?

; Common build settings across all devices
[env]
Expand Down Expand Up @@ -48,20 +56,23 @@ build_flags =
-I./src/MF_Modules
build_src_filter =
+<*>
-<./MF_CustomDevice>
extra_scripts =
pre:get_version.py
pre:get_CustomDevices.py

; Build settings for the Arduino Mega
[env:mega]
[env:mobiflight_mega]
platform = atmelavr
board = megaatmega2560
framework = arduino
build_flags =
${env.build_flags}
-DMF_CUSTOMDEVICE_SUPPORT=0
'-DMOBIFLIGHT_TYPE="MobiFlight Mega"'
-I./_Boards/Atmel/Board_Mega
build_src_filter =
${env.build_src_filter}
+<../_Boards/Atmel>
lib_deps =
${env.lib_deps}
${env.custom_lib_deps_Atmel}
Expand All @@ -70,16 +81,17 @@ extra_scripts =
${env.extra_scripts}

; Build settings for the Arduino Pro Micro
[env:micro]
[env:mobiflight_micro]
platform = atmelavr
board = sparkfun_promicro16
framework = arduino
build_flags =
${env.build_flags}
-DMF_CUSTOMDEVICE_SUPPORT=0
'-DMOBIFLIGHT_TYPE="MobiFlight Micro"'
-I./_Boards/Atmel/Board_ProMicro
build_src_filter =
${env.build_src_filter}
+<../_Boards/Atmel>
lib_deps =
${env.lib_deps}
${env.custom_lib_deps_Atmel}
Expand All @@ -89,16 +101,17 @@ extra_scripts =


; Build settings for the Arduino Uno
[env:uno]
[env:mobiflight_uno]
platform = atmelavr
board = uno
framework = arduino
build_flags =
${env.build_flags}
-DMF_CUSTOMDEVICE_SUPPORT=0
'-DMOBIFLIGHT_TYPE="MobiFlight Uno"'
-I./_Boards/Atmel/Board_Uno
build_src_filter =
${env.build_src_filter}
+<../_Boards/Atmel>
lib_deps =
${env.lib_deps}
${env.custom_lib_deps_Atmel}
Expand All @@ -107,16 +120,17 @@ extra_scripts =
${env.extra_scripts}

; Build settings for the Arduino Nano
[env:nano]
[env:mobiflight_nano]
platform = atmelavr
board = nanoatmega328
framework = arduino
build_flags =
${env.build_flags}
-DMF_CUSTOMDEVICE_SUPPORT=0
'-DMOBIFLIGHT_TYPE="MobiFlight Nano"'
-I./_Boards/Atmel/Board_Nano
build_src_filter =
${env.build_src_filter}
+<../_Boards/Atmel>
lib_deps =
${env.lib_deps}
${env.custom_lib_deps_Atmel}
Expand All @@ -125,25 +139,24 @@ extra_scripts =
${env.extra_scripts}

; Build settings for the Raspberry Pico original
[env:raspberrypico]
[env:mobiflight_raspberrypico]
platform = https://github.com/maxgerhardt/platform-raspberrypi.git
board = pico
framework = arduino
board_build.core = earlephilhower ; select new core
board_build.filesystem_size = 0M ; configure filesystem size. Default 0 Mbyte.
board_build.core = earlephilhower ; select new core
board_build.filesystem_size = 0M ; configure filesystem size. Default 0 Mbyte.
lib_ldf_mode = chain+
upload_protocol = mbed ; for debugging upoading can be changed to picoprobe
;debug_tool = picoprobe ; and uncomment this for debugging w/ picoprobe
upload_protocol = mbed ; for debugging upoading can be changed to picoprobe
;debug_tool = picoprobe ; and uncomment this for debugging w/ picoprobe
build_flags =
${env.build_flags}
-DUSE_INTERRUPT
-DMF_CUSTOMDEVICE_SUPPORT=0
'-DMOBIFLIGHT_TYPE="MobiFlight RaspiPico"'
-I./_Boards/RaspberryPi/Pico
-fpermissive
build_src_filter =
${env.build_src_filter}
+<../_Boards/RaspberryPi>
lib_deps =
${env.lib_deps}
monitor_speed = 115200
extra_scripts =
${env.extra_scripts}
${env.extra_scripts}
7 changes: 7 additions & 0 deletions src/CommandMessenger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@
#if MF_DIGIN_MUX_SUPPORT == 1
#include "DigInMux.h"
#endif
#if MF_CUSTOMDEVICE_SUPPORT == 1
#include "CustomDevice.h"
#endif

CmdMessenger cmdMessenger = CmdMessenger(Serial);
unsigned long lastCommand;
Expand Down Expand Up @@ -83,6 +86,10 @@ void attachCommandCallbacks()
cmdMessenger.attach(kSetShiftRegisterPins, OutputShifter::OnSet);
#endif

#if MF_CUSTOMDEVICE_SUPPORT == 1
cmdMessenger.attach(kSetCustomDevice, CustomDevice::OnSet);
#endif

#ifdef DEBUG2CMDMESSENGER
cmdMessenger.sendCmd(kDebug, F("Attached callbacks"));
#endif
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new callback for custom devices

Expand Down
Loading
Loading