CoretechR / OMOTE

Open Source Remote Using ESP32 and LVGL
https://hackaday.io/project/191752
GNU General Public License v3.0
1.02k stars 114 forks source link

Lay the ground work for a hardware abstraction layer #15

Closed MatthewColvin closed 11 months ago

MatthewColvin commented 11 months ago

This will allow the simulator and the actual esp32 to leverage the OmoteUI source code.

I am sure this functionally changes the hardware a lot but I am sure we can chat about this and how to clean that up.

Generally the idea is that any time we have a new platform that support lvgl we can extend OMOTE\Platformio\include\OmoteUI\HardwareAbstractionInterface.h this will tell the hardware how to do its job and then the UI will just be in place.

I don't really expect this PR to merge in but rather to just start up a conversation.

MatthewColvin commented 11 months ago

Hey @CoretechR how does the general abstraction look to you?

CoretechR commented 11 months ago

Hi @MatthewColvin, first of all, thank you so much for your help with this! This kind of abstraction and cleaning up the code is something I'm not so familiar with. I usually just throw all the code into one file and try to avoid scary things like pointers. So that's a great opportunity to learn. Just the GitHub actions alone are fantastic, I had no idea they worked that way.

Overall, I like the direction you are taking this. But I have to admit I don't completely understand everything yet in terms of the code. Is HardwareRevX planned as a placeholder for different hardware revisions? How would the HardwareAbstractionInterface work?

When running your code on my hardware, I had some problems at first. The functions inside HardwareRevX::init are rearranged. Calling restorePreferences before setting up the backlight causes a boot loop as the pwm output is not initialized yet. I would suggest this order of init functions:


  // Make sure ESP32 is running at full speed
  setCpuFrequencyMhz(240);
  wakeup_reason = getWakeReason();
  initIO();
  setupBacklight();

  Serial.begin(115200);

  restorePreferences();

  slowDisplayWakeup();
  setupTFT();
  setupTouchScreen();  
  initLVGL();

  setupIMU();
  setupIR();
}

After changing this, everything works apart from some graphical issues. The embedded images do not render for example. I will take a closer look into OmoteUI.

MatthewColvin commented 11 months ago

Hey @CoretechR Thanks for the feedback on my code. I want to make sure this is still following in the path you want.

My main goal was to get the HW aspects WIFI, IR, MQTT separated away from the UI. This will allow people to keep their forks up to date on the newest tweaks to the way we handle hardware events(such as library updates) but they can just keep their own OmoteUI.hpp in place and therefore their own "custom remote".

So info on that separation layer.

All that being said if you maybe wanted to make a branch for this in your repo that I could pull request to instead of the main branch then maybe we could stabilize and refine this based on what you want.

Hopefully we can make something that is super easy software wise for the end user :)

CoretechR commented 11 months ago

Thanks for fixing the init order, it now runs on my hardware! I like your concept of splitting the hardware, UI and main. That keeps everything more organized and will make future edits easier. It's important to me, that that the code is still maintainable and editable without requiring a strong programming background, at least until there's some high-level configuration tool. I am honestly having a hard time understanding the OmoteUI class. :sweat_smile:

There is a new "abstraction" branch. Can you change this pull request to the new branch?

MatthewColvin commented 11 months ago

Just move to the Abstraction Branch.

If you have any specific Questions about the OmoteUI totally shoot them my way. Also I can start a discussion to get others input too :)

This makes the ability to interface to the hardware completely defined by HardwareAbstractionInterface.h the which means the omoteUI does not have to care about setup and config and confusing library junk for all that hardware it can just call functions in the hardwareRevX or other implementations of that interface that have been overridden to actually do the work.

Admittedly this does limit the end user but that is why I want to really focus on the HardwareAbstractionInterface.h to make sure we support a good amount of features for the end users to use in the UI.

CoretechR commented 11 months ago

@MatthewColvin Hope it was alright that I merged this into the new branch.

MatthewColvin commented 11 months ago

@CoretechR awesome that is great I think maybe we can test off that branch as I keep it going.