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

Elral/issue148 #152

Merged
merged 13 commits into from
Feb 28, 2022
Merged

Elral/issue148 #152

merged 13 commits into from
Feb 28, 2022

Conversation

elral
Copy link
Collaborator

@elral elral commented Feb 8, 2022

Fixes #148

  1. mobiflight.cpp is splitted into several files, one per device plus commandmesenger.cpp and config.cpp.
  2. All device wrapper classes (e.g. Button.cpp) are initialized dynamically using the new deviveBuffer and there corrospending classes (e.g. MFBUtton.cpp) are intialized statically (but they will still initialized in the deviceBuffer). This gives the lowest RAM and flash usage. One exception is the MFStepper.cpp class, this class is still intialized dynamically to differ between FULL4WIRE and DRIVER support.
  3. The functionality of MFEEPROM.init() was moved to the constructor.
  4. #ifdef DEBUG was renamed to #ifdef DEBUG2CMDMESSENGER to avoid conflicts with other libraries (not in the used one, but maybe later in other libraries)
  5. The loop() is restructured, checking the time if a device must be updated is now done within the loop() and not the functions which are called before. I see the loop() as a kind of "controller".
  6. in all include files #ifndef xyz / #define xyz was replaced by #pragma once
  7. All files were checked if the include files are required. If they are not required they got deleted
  8. I hope I get it all

As I do not own Input- and Outputshifters I would highly appreciate if somebody could test it. All other devices are working. Also most changes from the last merging must be implemeted by hand.
I am still wondering about the uncommented PowerSaveOutputs(state); in the actual branch (what I left for now). Do outputs does not have a power saving?

@elral elral assigned neilenns and GioCC and unassigned neilenns and GioCC Feb 8, 2022
@elral elral requested review from neilenns and GioCC February 8, 2022 13:38
@neilenns
Copy link
Contributor

neilenns commented Feb 8, 2022

@elral This has a build failure:

src/CommandMessenger.cpp:3:10: fatal error: commandmessenger.h: No such file or directory

@elral
Copy link
Collaborator Author

elral commented Feb 8, 2022

@elral This has a build failure:

src/CommandMessenger.cpp:3:10: fatal error: commandmessenger.h: No such file or directory

Holy moly, (my?) PlatformIO does not consider lowercase / capitel letter. Thanks for your hint! I will change the file accordingly, otherwise I had to modify all files where it is included.

/edit: Strange, after renaming the file no changes show up which I could upload...

@neilenns
Copy link
Contributor

neilenns commented Feb 8, 2022

@elral Windows vs. Linux build environments I think.

Use the git mv command to rename it. See https://makolyte.com/how-to-change-the-casing-of-a-filename-when-using-git-on-windows/

@elral
Copy link
Collaborator Author

elral commented Feb 8, 2022

@neilenns do you have also a hint for me how to rename a folder? I didn't see a capitel "I" in ./src/MF_LCDDIsplay.
All time new surprises ;)

@neilenns
Copy link
Contributor

neilenns commented Feb 8, 2022

For folder renames it look like you might have to pass it through an intermediary folder:

git mv controller Controller-tmp
git mv Controller-tmp Controller

Current failure is this:

src/MF_Analog/Analog.cpp:6:10: fatal error: analog.h: No such file or directory

@elral
Copy link
Collaborator Author

elral commented Feb 8, 2022

@neilenns thanks a lot! I will care about tomorrow.

@elral
Copy link
Collaborator Author

elral commented Feb 9, 2022

I will create a new pull request after I opened issues for the changes which were also implemented (as per Neils recommendation)

@elral elral closed this Feb 9, 2022
@elral
Copy link
Collaborator Author

elral commented Feb 24, 2022

@MobiFlight-Admin @neilenns @GioCC
I checked again my pull request and plan to re-submit this one after discussions with Sebastian.
While going through all the files again (e.g. checking upper/lower case), there is still a topic I do not understand. Maybe someone can guide me on this.

Within the actual branch there is a function each for both Shifters:

  • OnInitInputShifter()
  • OnInitOutputShifter()

But as far as I can see both functions are never used, is this correct? Or do I simply not find the file where they are used?
Any help would be appreciated.

@GioCC
Copy link
Contributor

GioCC commented Feb 24, 2022

From what I can see, OnInitOutputShifter() was meant to be associated with a command, which however has never been defined; it can either be defined now, or the function can safely be removed.
I can't find OnInitInputShifter() though, and it would make little sense anyway (inputs need not be initialized, possibly read the first time, but that's done anyway).

@elral
Copy link
Collaborator Author

elral commented Feb 27, 2022

I can't find OnInitInputShifter() though, and it would make little sense anyway (inputs need not be initialized, possibly read the first time, but that's done anyway).

Ups, it's better to use copy and paste and to type it manually. It must be: OnInitInputShiftRegister which is also defined and not used.

@elral elral reopened this Feb 27, 2022
Copy link
Collaborator

@DocMoebiuz DocMoebiuz left a comment

Choose a reason for hiding this comment

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

BIG THANK YOU TO EVERYONE!!!

@DocMoebiuz DocMoebiuz merged commit b858b9f into MobiFlight:main Feb 28, 2022
@elral elral deleted the elral/issue148 branch March 10, 2022 05:42
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

Successfully merging this pull request may close these issues.

Split mobiflight.cpp into multiple files
4 participants