Refactor #5
Refactor #5
Conversation
- Removed unnecessary delay before reading values - Do not write register address if it was already written to INA219 before reading
Autodetection works better (at least on Windows)
- Sound handling through dedicated local class to reduce lines of code instead of directly using DacAudio low level actions - Screen handling factored a couple of commonly used code blocks into separate functions.
|
Hi hogthrob, I made some changes to your PR and uploaded them to your refactor repo. After you test it out please resubmit the PR with the revised files.
|
|
Will take a couple of days to retest it. Should have looked at the FOB key triggered announcements in the first place. |
I decided to revise some lazy code that I pasted into the updated misc.cpp. I'll upload the revision to your Refactor branch today. Removing the monitor_port directive from platformio.ini is a bit inconvenient for me, but I can understand why you did it. So I'm looking for a workaround: Do you know of a directive that forces monitor_port to use the Windows COMx found by the auto-detected upload_port? Something that could do this should make us both happy.
|
|
For me leaving out the monitor port makes my Windows machine automatically detect upload and monitor port as identical (which they are in case of ESP32 obviously). |
Well, you would think it was that easy. But apparently I live (and suffer) by Murphy's rules. In my installation the upload_port is correctly auto-detected, but monitor_port defaults to the wrong COMx. For example, upload uses COM4, but the serial terminal auto-detect incorrectly chooses COM5. The workaround for me was to use the platformio.ino directive to set the correct port. With the latest PR I'm back to typing the COMx value after each new flash. That's fun and games for awhile, until it's not.
|
|
Looked at your changes, tried them, made a very minor change (basically to get back to the original behavior when playing the promo message). So from my point of view we are good to go. Regarding the platformio.ini / COM port issue. You could do the same I had to do when the monitor_port was in: Changing the platformio.ini but never commiting this particular change. |
But your ini revision will be removing monitor_port when this PR is committed. I'm still trying to find a universally acceptable solution to restore the monitor_port. |
The current behavior is to wait at least SPLASH_TIME (2.5s), but this time can be much longer due to bluetooth scanning. The newly proposed change will further increase the splash's time before showing the home screen, which is not necessary. So would prefer that this change be reverted.
|
|
Ok, removed last commit. Regarding the port thing: If the assumption is correct, that most users don't have the problem you have and the implicit detection works for them, it is a service for those. If you don't like this, just change it back. |
|
Thanks, I'll commit your updated PR today.
I agree, specifying the monitor_port in the public release is undesirable. Unfortunately my Platformio IDE is being affected by an oddball bug in the IDE's port auto-detect code. So for now I will revert my local branch's platformio.ini while I patiently wait for a remedy from the developers.
|
|
Update: The Platformio developer has offered to include an enhancement patch for the Serial Monitor port issue that was causing me grief. The issue affects some workstation configurations with several generic USB ports. It will be released in Platformio 4.1.1. Details: platformio/platformio-core#3349
|
|
Thats great! So it was a fixable issue, good news. |
Improved INA219 response time
Removed monitor port
Sound and Screen Handling refactoring
A lot of changes, but no change in functionality.
Intention is to prepare for future UI and function additions (such as my WIP TIG Welding Support)