platformio - Githubissues

ramapcsx2 / gbs-control

GNU General Public License v3.0

809 stars 113 forks source link

platformio #551

Open way5 opened 7 months ago

way5 commented 7 months ago

platformio compatibility (latest arduino-framework, webSockets)
AsyncWebServer is replaced with standard solution for the sake of memory saving
WiFi module has been rewritten (excluded PersWiFiManager)
overal optimization, fixed backup/restore

"pio" branch still contain the beta version, a few features may not work as expected.

ramapcsx2 commented 7 months ago

Hey, I'm confused here. Didn't we just have a PIO thing? https://github.com/ramapcsx2/gbs-control/pull/507

way5 commented 7 months ago

Hi @ramapcsx2 . I my case it didn't worked, so I've fixed things a bit and I'd change them even more to order up, clean and prepare it to CI. If you, guys don't mind. However if this is not an objective of this project it's ok, just let me know. :) I'd also add a version to the firmware. Can you give me a hint where we are now in terms of versions? Thanks

ramapcsx2 commented 7 months ago

@Metaln00b Heya, could you share your thoughts?

way5 commented 7 months ago

I would also dare to:

move all the web things onto the flash (this is a headache to reflash the complete firmware every time you changing something in UI).
split the fat main.cpp into various modules
change the project file structure as per PIOs recommendations, and
implement one click build for the whole project or even better the CI/CD

Metaln00b commented 7 months ago

@Metaln00b Heya, could you share your thoughts?

Well, it has nothing to do with PlatformIO, at least not directly. But I see a lot of changes that might break the environment for the main developers. That's not to say that the changes are bad :P

If I see it correctly, debug messages were moved to the websocket, spiffs were changed to LittleFS, a few endpoints regarding AccessPoint were adjusted etc.

Metaln00b commented 7 months ago

I would also dare to:

move all the web things onto the flash (this is a headache to reflash the complete firmware every time you changing something in UI).

split the fat main.cpp into various modules

change the project file structure as per PIOs recommendations, and

implement one click build for the whole project or even better the CI/CD

Would it be okay for you to describe the pull request in more detail, what it changes?

Platformio is not the best headline right now 😅

But first of all, thank you for your support with this project🙏

Most developers are currently busy with other projects and can't look at the whole changes of code, so it would be very important to have a description of the changes, especially changes to folder structures etc.

way5 commented 7 months ago

Right. I am sorry for the vague description. In the beginning this branch was named "littlefs" since my initial objective was just to a bit change the source code in terms of 100% compatibility with PIO (such as silent build with no warnings, low memory-low flash :) , using latest framework maybe, just in one click) and change spiffs to littlefs.

Since my first attempt of the rapid dive into the large gbs-control.ino was not so rapid because of the high project entropy, I've decided to groom it a bit and that is how it all started.
The source code is actually rising the learning curve too much to allow people participating by doing small and rapid changes. Touching one thing pulls needless tests of other features.

No-offensive criticism here btw. :)

Now about the changes were made:

gbs-control.ino was converted into main.cpp and three additional modules were spawned:
- wserver.cpp - with all the webserver's stuffs
- wserial.cpp - supposedly all the serial and WS related functionality
- wifiman.cpp - replacement for an overloaded PersWiFiManager.cpp
SPIFFS -> LittleFS (since soon it becomes deprecated)
all (or at least the most) of the static data moved into flash (PGM)
some thing were reorganized in terms to be able to build with the latest arduino-framework
AsyncWebServer -> ESP8266WebServer (synchronous) it allowed to spare 3-5% of RAM.
platformio.ini been changed a bit.

I believe this is it... The only warning which is left for now with OLEDMenuItem, since sizeof nonexistent object is not allowed.

The only environmental problem I see here is that you cannot use Arduino IDE for building and uploading anymore, which is I believe even better since you're moving onto PIO which encourages you to use VScode instead.

ramapcsx2 commented 7 months ago

That all sounds reasonable, and I'm impressed someone went and started a cleanup here :p May I suggest that you treat this more like a fork, all depending on whether you want to continue working on it or whether you consider it done?

I'm open to merging, but if this is still in the make and break stages, then I'd prefer waiting a bit, so the way to build this doesn't change every 2 weeks, seemingly :)

ramapcsx2 commented 7 months ago

It's really good to split up the big .ino file. Please just try to not segment it all too much :)
SPIFFS -> LittleFS : sounds good. There is no preference, just whatever works is fine
Remember that static data in flash will incur some latencies. Sometimes it's good to have frequently used data in RAM
PIO / Arduino: Some people are used to their toolchains, and it would be great to keep compatibility with the main ones. If a pure Arduino and a pure PIO setup can both build the project, I'm good :)

way5 commented 7 months ago

That all sounds reasonable, and I'm impressed someone went and started a cleanup here :p May I suggest that you treat this more like a fork, all depending on whether you want to continue working on it or whether you consider it done?

That is not done yet definitely. I'd like to make some changes to ease the burden :) for all those who're going through the debris when they join the project. I agreed to leave it as a fork in my personal repo, so your repo would stay unmodified. However that would bring more ambiguity and confusion over there.

I'm open to merging, but if this is still in the make and break stages, then I'd prefer waiting a bit, so the way to build this doesn't change every 2 weeks, seemingly :)

My plan is to finish implementing a few more changes in this branch so it can be merged into the master and spawn a new one to fix the project structure there.

Let me know what do you think on that.

PS: taking into account your next comment on the arch and especially the two IDEs-type building possibility. I believe there is no problem with it especially because Arduino IDE is less demanding than PIO.

I also expect collaboration on that from other members of the project. If someone do not agree with something, I'd like to hear that before the big changes, and maybe return to the "yet another fork" version. ;)

nyanpasu64 commented 7 months ago

I'm a bit rusty in my understanding of the project, but:

There is currently no versioning system that I'm aware of.
Porting away from AsyncWebServer is a good idea (as it causes memory corruption and stack overflow from handling Wi-Fi operations in a deeply nested yield()), if you've made sure that handling one request at a time doesn't break any operations in the web UI (the more concurrent operations include saving/loading presets with/without per-slot filters, preset upload/download), and that the new server doesn't have the same issues.
If you switch build systems it's nice to add docs for the new system. You probably should fix the docs if you remove the old build system.

ramapcsx2 commented 6 months ago

What's been said. Feel free to go ahead and work on it, and as long as it still works at the end, there will be no problem merging it :)

way5 commented 6 months ago

I've been going through all the previous commits and I think there are two main milestones:

throughout the whole project is the video IO thing (adding, fixing, improving)
UI update, including web UI and user interaction schema with physical controls, wireless things

Thus in terms of versions I propose to assign version 3 (major) to the code in current branch. However, having in mind "keep it simple" principle I believe that would be more clearly to use the date of release markings for now, ex.: 24.0501 or 24.0502b (beta), etc

Please share your opinion.

way5 commented 6 months ago

I have a few questions related to the architecture:

Why there are userOptions and runtimeOptions and could they be merged in one structure of runtimeOptions?
What is the difference between rto->presetID and uopt->presetPreference?
What is the difference between Resolutions, Presets and Slots?
Why resolution change causes selecting custom preset?
Why "Presets" as a term can't be merged with Slots, so resolutions could be one of the options for each slot? ...

Concerning WebUI (master version):

Does backup/restore functionality works without an issue ?
Is there "remove preset" functionality currently in master?
Does OTA works without an issue ?
Does "reset/restore" functionality works properly? ...

~Thanks

ramapcsx2 commented 6 months ago

Oh gee, I'll try to remember..

user and runtime options: as the comments say, user holds the user's preferences, the selectable options. runtime holds (volatile) system variables, and could be better renamed to something like "activeState" or such. You can tell by the variables the 2 structs hold
rto->presetID / uopt->presetPreference: same thing here. the rto version is the active state and volatile, while the uopt version is the user's preference
Resolutions, Presets and Slots: relate to the actual resolution (currently active, i suppose), presets being a collection of common settings that are required to run any of the "preset" resolutions and slots.. are presets that get saved to user custom presets (in "slots")
Why resolution change causes selecting custom preset: Iirc this is a state the scaler is in, where settings are modified and functions need to work differently from the preset / known settings
Presets are the entire collection of all the register settings that allow one output resolution to work. You can not easily untangle a "preset" from a resolution. Instead, a preset is often used to dial in most settings, and then something custom may be added on top
webui questions: i'm out of the loop there, no idea :p

way5 commented 6 months ago

@ramapcsx2, so basically the designed process to store/load presets is as follows:

SLOTs are saved into slots.bin where each of 72 slots has structure: 1.1. SLOT_ID (char) 1.2. presetID - currently it's output resolution, I guess it is intended to be the first part of the preset file name (see. point 2) ... etc.
for each slot there are 9 types of configs created, which names formatted as input_video_format.SLOT_ID (must be presetID.SLOT_ID) are: 3.1. preset_ntsc.SLOT_ID 3.2.preset_pal.SLOT_ID 3.3. preset_ntsc_480p.SLOT_ID ...etc.
current/active SLOT_ID is going to be saved in preferencev2.txt file together with other user settings (saveUserPrefs()).

It feels like this is uncompleted intent to define the "many-to-many" dependency of "slots-presets". Also I cannot find "Selecting a resolution also makes it the new startup preset" functionality. So the reason why I got concerned about this is that what you see in UI is not what you get in this case. From the point of view of an ordinary user you want to open UI -> select config or change + save preferences -> load config -> be happy.

Is the preset file name input_video_format.SLOT_ID intended to allow the device automatically switch presets depending on the input video signal?

So I wonder if this would be the working schema.:

diag

Correct me if I am wrong...

ramapcsx2 commented 6 months ago

@nyanpasu64 Yo, I'm out of my field here. Is this how it's supposed to work? :p

ramapcsx2 commented 6 months ago

Just in general though, this whole system has clearly evolved over time, people requesting this or that feature, then others doing various consolidations and cleanups, as well as necessary changes for things like the OLED (iirc). It's become a hydra, and I don't think anyone would be too sour if it'd be redone. It all started as that idea of auto detecting one input and mapping to one of maybe 2 or 3 presets that work for that input.. :p

way5 commented 6 months ago

I think at this point we need to decide what is better to keep:

Fixed User presets inside of the slots
Dynamic preset identification, i.e.: 2.1. The device identifies input signal and creates a new slot with presets. 2.2. A user is able to adjust presets for this slot and change its name. However user is not able to create slots.

The approach 2 is highly limiting user experience since it rely on signal identification which is actually (signal) not tied to the source (ex: game console) itself. On the other hand if the user is able to switch slots itself. This leaves one point of decision making (user) at the time he needs to switch the slots.

ramapcsx2 commented 6 months ago

Note that people are generally fine with the capabilities of the preset system. It may be convoluted, but it seems to handle most things well enough :)

way5 commented 6 months ago

Alright, so I think we need to describe this functionality once and for all to avoid possible mistakes in functionality in future. The following diagram present one-to-one relation between slots and presets.

The device is detecting video input and uses corresponding preset without creating a new configuration file.
When a user decides to store the configuration, "slot-preset" pair is created containing all the necessary parameters.

diag

I'll make this available also in README.

nyanpasu64 commented 6 months ago

Is the preset file name input_video_format.SLOT_ID intended to allow the device automatically switch presets depending on the input video signal?

maybe? note i've been away from gbs-c development for months at this point.

switching to a custom slot does not load it until you load preset or power cycle. IIRC this is to allow you to save preset instead.
a custom slot can be associated with 1 or more custom (slot, input resolution) configurations. the intent is to allow a single custom slot to map to a different scaling configuration based on the input resolution. if a given input resolution's preset is missing, IIRC the gbs-c defaults to 960p output (assuming 480i/p input, no clue what happens with 576i/p input or HD).
the displayed output resolution on the GUI is determined by the most recently saved input resolution's scaling factor, and is cosmetic (the actual output resolution depends on the input resolution).
if you save a custom slot in passthrough, it "works" (after great effort on my part) but the gbs-c fails to detect input resolution changes and switch output scaling modes (IDK if this can be fixed, I don't know how input resolution detection works and if the necessary hardware registers aren't updated in passthrough mode).

is the current system good? it's probably not intuitive, but it works mostly.

sorry, i don't understand your diagrams or proposal:

Fixed User presets inside of the slots

Dynamic preset identification, i.e.: 2.1. The device identifies input signal and creates a new slot with presets. 2.2. A user is able to adjust presets for this slot and change its name. However user is not able to create slots.

Does backup/restore functionality works without an issue ?

mostly? backup streams individual files to the browser client which saves them to a file, and restore has the browser client separate these files and stream them to the server/ESP. (leftover files on the ESP are not removed when you restore a preset.)

this system most notably breaks when the buggy web server/etc. corrupts flash and creates files with duplicate or invalid filenames (#421).

Is there "remove preset" functionality currently in master?

i think?

Does OTA works without an issue ?

updating over USB works, i don't remember if updating over web works.

Does "reset/restore" functionality works properly?

i think (hope)? is restore the same as "backup/restore" you brought up earlier?

way5 commented 6 months ago

Is the preset file name input_video_format.SLOT_ID intended to allow the device automatically switch presets depending on the input video signal?

maybe? note i've been away from gbs-c development for months at this point.

Let's say "in theory", because it didn't work on my GBSC as I understood it should work.

switching to a custom slot does not load it until you load preset or power cycle. IIRC this is to allow you to save preset instead.

I've never seen this shared in a user manual or wherever. I was wondering before I put my hands on it why the device doesn't respond to configuration changes right away.

a custom slot can be associated with 1 or more custom (slot, input resolution) configurations. the intent is to allow a single custom slot to map to a different scaling configuration based on the input resolution. if a given input resolution's preset is missing, IIRC the gbs-c defaults to 960p output (assuming 480i/p input, no clue what happens with 576i/p input or HD).

Gonna try to wrap my head around...

the displayed output resolution on the GUI is determined by the most recently saved input resolution's scaling factor,

right, that is what I expect it to be.

and is cosmetic (the actual output resolution depends on the input resolution).

what do you mean by "cosmetic"? It does upscaling the input signal of 480p to 1080p when I have chosen 1080p or it depends ...?

if you save a custom slot in passthrough, it "works" (after great effort on my part) but the gbs-c fails to detect input resolution changes and switch output scaling modes (IDK if this can be fixed, I don't know how input resolution detection works and if the necessary hardware registers aren't updated in passthrough mode).

yes, I've seen that code. The data/manual about registers was ever available somewhere?

is the current system good? it's probably not intuitive, but it works mostly.

mostly is a keyword :)

sorry, i don't understand your diagrams or proposal:

never mind for now. I am trying to find a workable configuration logic that matches the product requirements. This is not just my wish to change something, it is because my device doesn't work as I believe it should be. The last diagram displays that it's maybe better to attach the only preset to a slot, so ONE slot - ONE preset config. However I have a doubt now that it would be a correct logic.

Fixed User presets inside of the slots

Dynamic preset identification, i.e.: 2.1. The device identifies input signal and creates a new slot with presets. 2.2. A user is able to adjust presets for this slot and change its name. However user is not able to create slots.

Does backup/restore functionality works without an issue ?

mostly? backup streams individual files to the browser client which saves them to a file, and restore has the browser client separate these files and stream them to the server/ESP. (leftover files on the ESP are not removed when you restore a preset.)

this system most notably breaks when the buggy web server/etc. corrupts flash and creates files with duplicate or invalid filenames (#421).

Didn't work for me also. That is what I'd like to fix after figuring out slots-presets relations.

Does OTA works without an issue ?

updating over USB works, i don't remember if updating over web works.

Didn't work either. Upload only via USB...

Is there a description of rto->videoStandardInput parameter values? I have identified just a few values for now.

nyanpasu64 commented 6 months ago

what do you mean by "cosmetic"? It does upscaling the input signal of 480p to 1080p when I have chosen 1080p or it depends ...?

If you enable a 480i input, load the 1080p output preset, and save over slot A, then slot A will scale 240p/480i (15khz) to 1080p. If you then enable a 480p (31khz) input, load the 1080p output preset, and save again over slot A, then slot A will scale both 15khz and 31khz to 1080p. If you instead load the passthrough preset and save over slot A, then slot A will scale 15khz to 1080p but passthrough 31khz input signals. The label on the UI button reflects the most recently saved output resolution across all input resolutions, but in that slot, each input resolution has an independent output resolution/scaling mode.

Is this a good design? It's useful to scale 15 and 31 kHz signals differently (eg. on a VGA monitor, 960p with virtual scanlines for 15khz but native 480p), but the current UI was confusing to me at first (and probably new users in general).

I think the design was originally implemented this way because even at the same output resolution, different input resolutions require different register contents that can be further modified using the UI movement/scaling buttons (and saving a preset is mostly a register dump/load, though with some additional work performed upon load to stabilize the chip).

ramapcsx2 commented 6 months ago

I think the design was originally implemented this way because even at the same output resolution, different input resolutions require different register contents that can be further modified using the UI movement/scaling buttons (and saving a preset is mostly a register dump/load, though with some additional work performed upon load to stabilize the chip).

Exactly :)

nyanpasu64 commented 6 months ago

can't get it to build (with vscode platformio) to test.

[ ] on Windows, you must with open(os.getcwd() + '/translation.json', 'r', encoding='utf-8') as data: or else you'll get a character encoding error reading a UTF-8 file as text.
[ ] the build still fails because it's trying to open some Arial file that isn't in this branch now:

Traceback (most recent call last):
  File "C:\Users\user\code\gbs-c\gbs-control/scripts/generate_translations.py", line 130, in <module>
    raise FileNotFoundError(f"Font does not exist: {font}") from None
FileNotFoundError: Font does not exist: Arial

way5 commented 6 months ago

I don't have a windows setup available for testing at this very moment, but I've added encoding='utf-8' parameter. Python doesn't recognize the font name, however it is in defaults list: https://learn.microsoft.com/en-us/typography/fonts/windows_10_font_list Let's find out some default font which is installed on all systems OR the choice maybe given to a user which is going to compile the project.

way5 commented 6 months ago

@nyanpasu64 I wonder why Bypass mode is not actually Passthrough? Out of all the 3 bypass modes OutputBypass, PresetHdBypass, PresetBypassRGBHV no one is transferring video from input to output without changes. In fact there is no video at all on any of these modes. Is there a tricky logic which I didn't see or it is intentional behavior?
In addition, is there any info for reference on PLC registers? I'd like to understand a few things.

ramapcsx2 commented 6 months ago

Bypass is an operating mode where the ADC results get transported to the output unit without passing most digital blocks. It doesn't use the SDRAM, doesn't scale, etc, and the mode is actually called bypass in the various registers for it. What still happens is sync extraction and forwarding to the output, where they become CSync, iirc. There are limitations on what this can do, and if you don't get a picture, most likely your display doesn't like the limitations.

You can find all the documentation that exists on the google drive link in the first post here: https://shmups.system11.org/viewtopic.php?f=6&t=52172

ramapcsx2 commented 5 months ago

My my, I notice the changes are getting very extensive. You're retooling everything, right @way5 ?

Please try to get some feedback from someone available / willing to test, see where this is going, if you can :)

way5 commented 5 months ago

Hey! 😄 I was about to say that at this moment I'd appreciate your feedback on it. The work is not done, however it is kind of a milestone.

Say it is very close to what I wanted to have in my GBSc from the factory.

Despite it looks like too many changes my plan is to keep things native, at least on a critical path. 🙂 Most things are reimplemented and improved.

There will be an excerpt of all the changes with the next commit.

ramapcsx2 commented 5 months ago

Ah, again, I'm super busy with my xStation project atm. I can't spend any time on anything else really, I'm sorry :p

Generally, I don't doubt your changes. It's about whether your choices are what other people expect from the scaler, whether things you may have removed were actually used by people, etc.

There's also things that need test time, where people simply use the scaler normally, and report back any eventual new problems. grafik Things like those yield() removals could be good, could be bad.. They were there for a reason once, but that might not apply anymore, and it takes testing to know.

Anyway, I think this is looking great overall. We need some people to check it out next :)

nyanpasu64 commented 5 months ago

I have not been able to successfully build and upload this PR to my GBS-Control unit. (My GBS-C ESP8266 is connected over USB to a PC running Arch Linux VS Code with the PlatformIO plugin.)

Cannot upload binary to ESP8266 on Linux

When I open the PlatformIO sidebar and click Upload, I get an error accompanied by a series of garbled serial commands in the web UI:

 *  Executing task in folder gbs-control: platformio run --target upload --environment d1_mini 

Processing d1_mini (platform: espressif8266; board: d1_mini; framework: arduino)
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Verbose mode can be enabled via `-v, --verbose` option

🍺: generating locale data (en-US)

Finished. Output file: /home/nyanpasu64/code/gbs-c/gbs-control/src/OLEDMenuTranslations.h

🍺: running build of v.24.0528

CONFIGURATION: https://docs.platformio.org/page/boards/espressif8266/d1_mini.html
PLATFORM: Espressif 8266 (4.1.0) > WeMos D1 R2 and mini
HARDWARE: ESP8266 160MHz, 80KB RAM, 4MB Flash
PACKAGES: 
 - framework-arduinoespressif8266 @ 3.30101.0 (3.1.1) 
 - tool-esptool @ 1.413.0 (4.13) 
 - tool-esptoolpy @ 1.30000.201119 (3.0.0) 
 - tool-mklittlefs @ 1.203.210628 (2.3) 
 - tool-mkspiffs @ 1.200.0 (2.0) 
 - toolchain-xtensa @ 2.100300.220621 (10.3.0)
LDF: Library Dependency Finder -> https://bit.ly/configure-pio-ldf
LDF Modes: Finder ~ chain, Compatibility ~ soft
Found 41 compatible libraries
Scanning dependencies...
Dependency Graph
|-- WebSockets @ 2.4.1+sha.93707d4
|-- Si5351mcu @ 0.7.1+sha.cbbd806
|-- ESP8266 and ESP32 OLED driver for SSD1306 displays @ 4.6.1+sha.ed1feac
|-- ESP8266-ping @ 2.0.1+sha.a9ab808
|-- Wire @ 1.0
|-- LittleFS @ 0.1.0
|-- ESP8266WiFi @ 1.0
|-- ArduinoOTA @ 1.0
|-- DNSServer @ 1.1.1
|-- ESP8266WebServer @ 1.0
|-- ESP8266mDNS @ 1.2
Building in release mode
Retrieving maximum program size .pio/build/d1_mini/firmware.elf
Checking size .pio/build/d1_mini/firmware.elf
Advanced Memory Usage is available via "PlatformIO Home > Project Inspect"
RAM:   [======    ]  58.3% (used 47780 bytes from 81920 bytes)
Flash: [=====     ]  52.4% (used 547351 bytes from 1044464 bytes)
Configuring upload protocol...
AVAILABLE: espota, esptool
CURRENT: upload_protocol = esptool
Looking for upload port...
Auto-detected: /dev/ttyUSB0
Uploading .pio/build/d1_mini/firmware.bin
esptool.py v3.0
Serial port /dev/ttyUSB0
Connecting........_____....._____....._____....._____....._____....._____....._____

A fatal error occurred: Failed to connect to ESP8266: Invalid head of packet (0x21)
*** [upload] Error 2
================================================================================================== [FAILED] Took 8.73 seconds ==================================================================================================

 *  The terminal process "platformio 'run', '--target', 'upload', '--environment', 'd1_mini'" terminated with exit code: 1. 
 *  Terminal will be reused by tasks, press any key to close it.

The web UI shows something like:

serial command (null byte, cannot be copied) at settings source 2, custom slot 66, status 0
serial command (null byte, cannot be copied) at settings source 2, custom slot 66, status 0
serial command (null byte, cannot be copied) at settings source 2, custom slot 66, status 0
serial command (null byte, cannot be copied) at settings source 2, custom slot 66, status 0
serial command ~ at settings source 2, custom slot 66, status 0
<reset>
Scanning inputs for sources ...
serial command f at settings source 2, custom slot 66, status 0
peaking off
serial command f at settings source 2, custom slot 66, status 0
peaking on
serial command f at settings source 2, custom slot 66, status 0
peaking off
...

I can still build and properly upload to my ESP using Arduino IDE in "NodeMCU 1.0" mode.

Interestingly, on master I can actually successfully upload using the old platformio.ini:

Advanced Memory Usage is available via "PlatformIO Home > Project Inspect"
RAM:   [======    ]  60.9% (used 49856 bytes from 81920 bytes)
Flash: [========  ]  77.2% (used 806244 bytes from 1044464 bytes)
Creating BIN file ".pio/build/d1_mini/firmware.bin" using "/home/nyanpasu64/.platformio/packages/framework-arduinoespressif8266@3.20704.0/bootloaders/eboot/eboot.elf" and ".pio/build/d1_mini/firmware.elf"
Configuring upload protocol...
AVAILABLE: espota, esptool
CURRENT: upload_protocol = esptool
Looking for upload port...
Auto-detected: /dev/ttyUSB0
Uploading .pio/build/d1_mini/firmware.bin
esptool.py v3.0
Serial port /dev/ttyUSB0
Connecting....
Chip is ESP8266EX
Features: WiFi
Crystal is 26MHz
MAC: e0:98:06:16:5e:e0
Uploading stub...
Running stub...
Stub running...
Changing baud rate to 921600
Changed.
Configuring flash size...
Compressed 810400 bytes to 589125...
...

I think upload_resetmethod = ck is not properly resetting the GBS-C into flash mode prior to sending a firmware update package (causing it to be interpreted as serial commands instead). Removing this line allows the package to upload (at the frighteningly low baud rate of 57600, I normally use 460800 on Linux and ~~576000~~ EDIT: 512000 on Windows, I don't know why Arduino IDE exposes a different list of baud rates per OS, I'd increase upload_speed = 57600 as well?).

Web UI not working

After removing upload_resetmethod = ck, I tried uploading this project. Upon visiting http://gbscontrol.local/, I get a Firefox error:

Content Encoding Error

The page you are trying to view cannot be shown because it uses an invalid or unsupported form of compression.

Please contact the website owners to inform them of this problem.

I tried debugging with curl, and got this magnificent HTTP/1.1 200 OK gzip archive consisting of a 404 error:

nyanpasu64@ryzen ~ [2]> curl -v http://gbscontrol.local/
* Host gbscontrol.local:80 was resolved.
* IPv6: (none)
* IPv4: 192.168.0.87
*   Trying 192.168.0.87:80...
* Connected to gbscontrol.local (192.168.0.87) port 80
> GET / HTTP/1.1
> Host: gbscontrol.local
> User-Agent: curl/8.8.0
> Accept: */*
> 
* Request completely sent off
< HTTP/1.1 200 OK
< Content-Type: text/html
< Content-Encoding: gzip
< Content-Length: 49
< Connection: keep-alive
< Keep-Alive: timeout=2000
< 
* Connection #0 to host gbscontrol.local left intact
<p style="align:center;">404 | Page Not Found</p>⏎                                                                                            nyanpasu64@ryzen ~>

Scaling results

When I power on my Wii, the resulting image is 1280x960 and slightly shifted to the right compared to a regular PC image or the previous GBS-C firmware.

After restarting the GBS-Control, I get no video signal detected by my CRT monitor over VGA. If I turn off the Wii, power-cycle the GBS-C, then turn on the Wii, I get no video signal and the serial monitor shows output:

Activity detected, input: Component
video mode counter: 2
video mode counter: 3
video mode counter: 4
video mode counter: 5
video mode counter: 6
video mode counter: 7
video mode counter: 8

Format change: 3 <stable>
FrameSyncManager::cleanup(), reset video frequency
ntsc_240p is now active
video mode set: 3
ADC offset: R: 0x0043 G: 0x0043 B: 0x0042
FrameSyncManager::cleanup(), reset video frequency
vsyncPeriodAndPhase(), TEST_BUS_SEL=0
HTotal Adjust: -5, source Hz: 59.939, output Hz: 59.947
Output Format: Component

preset applied: 240p for EDTV 60Hz

Git nitpicks

It seems that building in PlatformIO regenerates builds/firmware.bin, gbs-control.ino, and src/OLEDMenuTranslations.h, all of which are currently checked into Git? Additionally "Build Filesystem Image" modifies checked-in file builds/filesystem.bin. This results in modified files, preventing switching branches unless I reset changes in my working directory. I'd say this would noticeably impede development (at least the way I normally work on other projects).

Even though gbs-control.ino is regenerated, I cannot build this file in Arduino IDE anymore, with error:

In file included from /home/nyanpasu64/code/gbs-c/gbs-control/gbs-control.ino:31:0:
/home/nyanpasu64/code/gbs-c/gbs-control/src/wserial.h:16:30: fatal error: WebSocketsServer.h: No such file or directory
 #include <WebSocketsServer.h>
                              ^
compilation terminated.
exit status 1
Error compiling for board NodeMCU 1.0 (ESP-12E Module).

Is this file passed into the PlatformIO compilers, or just a vestigial byproduct that's generated (from main.cpp and others) but not useful for either PlatformIO or Arduino IDE?

I have not reviewed the code for quality.

way5 commented 5 months ago

@nyanpasu64 thanks for your feedback. I'll check that asap

ramapcsx2 commented 5 months ago

Note that image shifts and such might be caused by not running any of the "post-preset" functions, or generally skipping functions that would modify parameters based on the current environment. To make it easier, you could compare full register dumps between old and new firmware, see where there are changes, and then work back from those to find which function (used to) set those regs.

way5 commented 5 months ago

@ramapcsx2, the code on image you've published belongs to the previous builds where I was chasing memory leak. This is work in progress, some artifacts may be found occasionally in code, these were used for measuring delays etc., please omit that inclusions. 😉

@nyanpasu64, despite the flash memory map doesn't change, there were quite a few changes which imply a clean install. It is your decision to do that but I'd highly recommend do erase the whole flash.

Let me know if your board is somewhat different than mine:

gbsc-board

Building in ArduinoIDE

Please refer to README in pio branch. You need to delete src/main.cpp in order to be able to successfully build the sketch. In my case it has been built flawlessly with Arduino 1.8.19.

. Variables and constants in RAM (global, static), used 47876 / 80192 bytes (59%)
║   SEGMENT  BYTES    DESCRIPTION
╠══ DATA     2624     initialized variables
╠══ RODATA   4404     constants       
╚══ BSS      40848    zeroed variables
. Instruction RAM (IRAM_ATTR, ICACHE_RAM_ATTR), used 62399 / 65536 bytes (95%)
║   SEGMENT  BYTES    DESCRIPTION
╠══ ICACHE   32768    reserved space for flash instruction cache
╚══ IRAM     29631    code in IRAM    
. Code in flash (default, ICACHE_FLASH_ATTR), used 520504 / 1048576 bytes (49%)
║   SEGMENT  BYTES    DESCRIPTION
╚══ IROM     520504   code in flash

Cannot upload binary to ESP8266 on Linux - your Linux machine seems to be unable to establish connection with ESP. Let me know if you could upload it eventually while on pio branch last commit. There a few changes in platformio.ini but none of them could affect the connectivity I believe. I'll leave this to the near future test. It works well for me on Ubuntu 24.04 / Win10 (Python 3.12, Arduino 1.8.19).

Erasing flash

Processing d1_mini (platform: espressif8266; board: d1_mini; framework: arduino)
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Verbose mode can be enabled via `-v, --verbose` option

🍺: generating locale data (en-US)

Output file: /Volumes/Stotage/Workspace/gbs-control/src/OLEDMenuTranslations.h

🍺: running build of v.24.0602

CONFIGURATION: https://docs.platformio.org/page/boards/espressif8266/d1_mini.html
PLATFORM: Espressif 8266 (4.2.1) > WeMos D1 R2 and mini
HARDWARE: ESP8266 160MHz, 80KB RAM, 4MB Flash
PACKAGES: 
 - framework-arduinoespressif8266 @ 3.30102.0 (3.1.2) 
 - tool-esptool @ 1.413.0 (4.13) 
 - tool-esptoolpy @ 1.30000.201119 (3.0.0) 
 - toolchain-xtensa @ 2.100300.220621 (10.3.0)
LDF: Library Dependency Finder -> https://bit.ly/configure-pio-ldf
LDF Modes: Finder ~ chain, Compatibility ~ soft
Found 41 compatible libraries
Scanning dependencies...
Dependency Graph
|-- WebSockets @ 2.4.1+sha.93707d4
|-- Si5351mcu @ 0.7.1+sha.cbbd806
|-- ESP8266 and ESP32 OLED driver for SSD1306 displays @ 4.5.0+sha.d3b23d0
|-- ESP8266-ping @ 2.0.1+sha.a9ab808
|-- Wire @ 1.0
|-- LittleFS @ 0.1.0
|-- ESP8266WiFi @ 1.0
|-- ArduinoOTA @ 1.0
|-- DNSServer @ 1.1.1
|-- ESP8266WebServer @ 1.0
|-- ESP8266mDNS @ 1.2
Building in release mode
Looking for serial port...
Auto-detected: /dev/cu.usbserial-1410
Erasing...
esptool.py v3.0
Serial port /dev/cu.usbserial-1410
Connecting....
Chip is ESP8266EX
Features: WiFi
Crystal is 26MHz
MAC: ac:0b:XX:XX:XX:XX
Uploading stub...
Running stub...
Stub running...
Erasing flash (this may take a while)...
Chip erase completed successfully in 12.9s
Hard resetting via RTS pin...
============================================================================ [SUCCESS] Took 21.74 seconds ============================================================================
 *  Terminal will be reused by tasks, press any key to close it.

Firmware uploading

Processing d1_mini (platform: espressif8266; board: d1_mini; framework: arduino)
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Verbose mode can be enabled via `-v, --verbose` option

🍺: generating locale data (en-US)

Output file: /Volumes/Stotage/Workspace/gbs-control/src/OLEDMenuTranslations.h

🍺: running build of v.24.0602

CONFIGURATION: https://docs.platformio.org/page/boards/espressif8266/d1_mini.html
PLATFORM: Espressif 8266 (4.2.1) > WeMos D1 R2 and mini
HARDWARE: ESP8266 160MHz, 80KB RAM, 4MB Flash
PACKAGES: 
 - framework-arduinoespressif8266 @ 3.30102.0 (3.1.2) 
 - tool-esptool @ 1.413.0 (4.13) 
 - tool-esptoolpy @ 1.30000.201119 (3.0.0) 
 - tool-mklittlefs @ 1.203.210628 (2.3) 
 - tool-mkspiffs @ 1.200.0 (2.0) 
 - toolchain-xtensa @ 2.100300.220621 (10.3.0)
LDF: Library Dependency Finder -> https://bit.ly/configure-pio-ldf
LDF Modes: Finder ~ chain, Compatibility ~ soft
Found 41 compatible libraries
Scanning dependencies...
Dependency Graph
|-- WebSockets @ 2.4.1+sha.93707d4
|-- Si5351mcu @ 0.7.1+sha.cbbd806
|-- ESP8266 and ESP32 OLED driver for SSD1306 displays @ 4.5.0+sha.d3b23d0
|-- ESP8266-ping @ 2.0.1+sha.a9ab808
|-- Wire @ 1.0
|-- LittleFS @ 0.1.0
|-- ESP8266WiFi @ 1.0
|-- ArduinoOTA @ 1.0
|-- DNSServer @ 1.1.1
|-- ESP8266WebServer @ 1.0
|-- ESP8266mDNS @ 1.2
Building in release mode
Retrieving maximum program size .pio/build/d1_mini/firmware.elf
Checking size .pio/build/d1_mini/firmware.elf
Advanced Memory Usage is available via "PlatformIO Home > Project Inspect"
RAM:   [=====     ]  50.2% (used 41124 bytes from 81920 bytes)
Flash: [=====     ]  52.4% (used 547527 bytes from 1044464 bytes)
Configuring upload protocol...
AVAILABLE: espota, esptool
CURRENT: upload_protocol = esptool
Looking for upload port...
Auto-detected: /dev/cu.usbserial-1410
Uploading .pio/build/d1_mini/firmware.bin
esptool.py v3.0
Serial port /dev/cu.usbserial-1410
Connecting....
Chip is ESP8266EX
Features: WiFi
Crystal is 26MHz
MAC: ac:0b:XX:XX:XX:XX
Uploading stub...
Running stub...
Stub running...
Configuring flash size...
Compressed 551680 bytes to 370288...
Writing at 0x00000000... (4 %)
Writing at 0x00004000... (8 %)
Writing at 0x00008000... (13 %)
Writing at 0x0000c000... (17 %)
Writing at 0x00010000... (21 %)
Writing at 0x00014000... (26 %)
Writing at 0x00018000... (30 %)
Writing at 0x0001c000... (34 %)
Writing at 0x00020000... (39 %)
Writing at 0x00024000... (43 %)
Writing at 0x00028000... (47 %)
Writing at 0x0002c000... (52 %)
Writing at 0x00030000... (56 %)
Writing at 0x00034000... (60 %)
Writing at 0x00038000... (65 %)
Writing at 0x0003c000... (69 %)
Writing at 0x00040000... (73 %)
Writing at 0x00044000... (78 %)
Writing at 0x00048000... (82 %)
Writing at 0x0004c000... (86 %)
Writing at 0x00050000... (91 %)
Writing at 0x00054000... (95 %)
Writing at 0x00058000... (100 %)
Wrote 551680 bytes (370288 compressed) at 0x00000000 in 71.4 seconds (effective 61.8 kbit/s)...
Hash of data verified.

Leaving...
Soft resetting...
============================================================================ [SUCCESS] Took 81.94 seconds ============================================================================
 *  Terminal will be reused by tasks, press any key to close it.

Filesystem image uploading

Processing d1_mini (platform: espressif8266; board: d1_mini; framework: arduino)
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Verbose mode can be enabled via `-v, --verbose` option

🍺: generating locale data (en-US)

Output file: /Volumes/Stotage/Workspace/gbs-control/src/OLEDMenuTranslations.h

🍺: running build of v.24.0602

CONFIGURATION: https://docs.platformio.org/page/boards/espressif8266/d1_mini.html
PLATFORM: Espressif 8266 (4.2.1) > WeMos D1 R2 and mini
HARDWARE: ESP8266 160MHz, 80KB RAM, 4MB Flash
PACKAGES: 
 - framework-arduinoespressif8266 @ 3.30102.0 (3.1.2) 
 - tool-esptool @ 1.413.0 (4.13) 
 - tool-esptoolpy @ 1.30000.201119 (3.0.0) 
 - tool-mklittlefs @ 1.203.210628 (2.3) 
 - tool-mkspiffs @ 1.200.0 (2.0) 
 - toolchain-xtensa @ 2.100300.220621 (10.3.0)
LDF: Library Dependency Finder -> https://bit.ly/configure-pio-ldf
LDF Modes: Finder ~ chain, Compatibility ~ soft
Found 41 compatible libraries
Scanning dependencies...
Dependency Graph
|-- WebSockets @ 2.4.1+sha.93707d4
|-- Si5351mcu @ 0.7.1+sha.cbbd806
|-- ESP8266 and ESP32 OLED driver for SSD1306 displays @ 4.5.0+sha.d3b23d0
|-- ESP8266-ping @ 2.0.1+sha.a9ab808
|-- Wire @ 1.0
|-- LittleFS @ 0.1.0
|-- ESP8266WiFi @ 1.0
|-- ArduinoOTA @ 1.0
|-- DNSServer @ 1.1.1
|-- ESP8266WebServer @ 1.0
|-- ESP8266mDNS @ 1.2
Building in release mode
before_buildfs([".pio/build/d1_mini/littlefs.bin"], ["data"])

🍺: building WebUI

npm run build

> gbscontrol-webui@1.2.0 build
> tsc --project ./tsconfig.json && cd ./public/scripts && node ./build.js && ./html2h.sh

WebUI ready to compress
index.html GENERATED

Building file system image from 'data' directory to .pio/build/d1_mini/littlefs.bin
/__index
Looking for upload port...
Auto-detected: /dev/cu.usbserial-1410
Uploading .pio/build/d1_mini/littlefs.bin
esptool.py v3.0
Serial port /dev/cu.usbserial-1410
Connecting....
Chip is ESP8266EX
Features: WiFi
Crystal is 26MHz
MAC: ac:0b:XX:XX:XX:XX
Uploading stub...
Running stub...
Stub running...
Configuring flash size...
Compressed 1024000 bytes to 200754...
Writing at 0x00300000... (7 %)
Writing at 0x00304000... (15 %)
Writing at 0x00308000... (23 %)
Writing at 0x0030c000... (30 %)
Writing at 0x00310000... (38 %)
Writing at 0x00314000... (46 %)
Writing at 0x00318000... (53 %)
Writing at 0x0031c000... (61 %)
Writing at 0x00320000... (69 %)
Writing at 0x00324000... (76 %)
Writing at 0x00328000... (84 %)
Writing at 0x0032c000... (92 %)
Writing at 0x00330000... (100 %)
Wrote 1024000 bytes (200754 compressed) at 0x00300000 in 38.7 seconds (effective 211.4 kbit/s)...
Hash of data verified.

Leaving...
Soft resetting...
============================================================================ [SUCCESS] Took 54.41 seconds ============================================================================
 *  Terminal will be reused by tasks, press any key to close it.

Web UI not working - obviously you need to upload project correctly without errors to be able to get it work and be testing something.

curl -v http://gbscontrol.local/                                    venv 20:57
*   Trying 192.168.1.100...
* TCP_NODELAY set
* Connected to gbscontrol.local (192.168.1.100) port 80 (#0)
> GET / HTTP/1.1
> Host: gbscontrol.local
> User-Agent: curl/7.64.1
> Accept: */*
> 
< HTTP/1.1 200 OK
< Content-Type: text/html
< Content-Encoding: gzip
< Content-Length: 198390
< Connection: keep-alive
< Keep-Alive: timeout=2000
< 
Warning: Binary output can mess up your terminal. Use "--output -" to tell 
Warning: curl to output it to your terminal anyway, or consider "--output 
Warning: <FILE>" to save to a file.
* Failed writing body (0 != 1460)
* Closing connection 0

My device works well with Firefox, Chrome, Safari. However if you simultaneously open WebUI in different browsers it may present lags on WS update queries or may fail to load. Sometimes GBSC is not announcing itself on my network fast enough, especially after frequent restarts, so using curl right after re-flashing may lead to a timeout. Anyways, let's start with a proper installation here.

Scaling results - I need some testing here. Will come back later to this topic. There are a few features that doesn't work since master. Never the less thanks for the hint
Git nitpicks - well, this is WIP branch so I've added builds directory in order to be able to access immediately to the latest builds for testing. Obviously I didn't meant to leave it for production. I'd manage it in future with releases. builds dir. is no longer included since last commit. The header src/OLEDMenuTranslations.h must be updated every time you build the project in PIO, it could be made smarter - checking if translations.json was updated -> TODO.
I have not reviewed the code for quality - not needed for now, however I am doing it all along, the old code was not reviewed I believe. This is the next step after casting out all bugs.

ramapcsx2 commented 5 months ago

I can confirm one odd thing about pretty much all Espressif platforms: Their reset circuitry is a recommendation, which is often not followed correctly by every implementation, and so their software to toggle the reset has seen multiple tweaks and versions over time. Reset to upload is simply shaky every time. Find a setting that works, and it'll be good. But change too much (different USB port, different devkit, etc), and another setting might be needed. Simply switch the upload baud rate setting a bit around, and I bet one other one will work :)

nyanpasu64 commented 5 months ago

I am not using a prebuilt GBS-C, but a self-assembled unit with a NodeMCU v3.x.

PXL_20240603_060830858

In Arduino IDE, I upload with the NodeMCU v1.0 board configuration. On your branch, if I change platformio.ini to upload_resetmethod = nodemcu (tested on Windows) or remove the line (tested on Linux), I can upload to the board from PlatformIO.

Building and uploading the firmware

After pulling your latest branch changes (bf8aebf57c6fa3ba16ae055cb138a90ad16b37c0, minus your latest commit), I can build on Windows now.

I didn't know the filesystem had to be uploaded separately from the firmware. I had assumed it was unchanged from master, where the web UI's source code is embedded as byte arrays into the firmware image, and the program is fully functional when the flash filesystem is cleared (files are written as you change settings or save presets).

Does this mean that users will have more steps to flash a working GBS-Control image?
Will changes to the web UI's HTML file require flashing a new filesystem (I think containing file __index)? This erases saved presets and settings, which I don't think is a pleasant development experience (if I'm working on Web UI changes, I'd want to keep saved presets), though end users flash firmware less often.
- To workaround this as a user, you can save all presets and reupload them after flashing a clean filesystem image. Maybe this is safer than "patching" the existing filesystem image with new system files (which could accumulate latent filesystem corruption like #421 and eventually break both saved presets and system files). It's still extra work that could be avoided by storing index.html in the firmware image directly.
  - I hope this PR prevents #421 from happening anymore, but we'll have to see if it still pops up during usage.
I'm concerned that storing system files on the filesystem instead of the firmware image makes them vulnerable to corruption during flash writes. It also allows the web UI to desynchronize with the underlying firmware, which can result in partial firmware updates (where the C++ code is uploaded but the web UI isn't) and broken systems if the communication protocol between the browser and firmware changes.
- Is there a specific reason you didn't leave index.html as part of the firmware image?

Unfortunately, on Windows "Upload Filesystem Image" generates an error and the GBS-C webserver sends the same 404 error as before.

WebUI ready to compress
'.' is not recognized as an internal or external command,
operable program or batch file.
*** Error 1
Building file system image from 'data' directory to .pio\build\d1_mini\littlefs.bin
/webui.html

I think this error results from running package.json's "build": "tsc --project ./tsconfig.json && cd ./public/scripts && node ./build.js && ./html2h.sh" in a cmd shell, which interprets a / as a flag for a program rather than a path separator. I do not know how to make the same shell command work on both Linux and Windows (perhaps look into https://www.google.com/search?q=package.json+scripts+windows+linux ?).

If I run this shell script manually from Git Bash, then "Upload Filesystem Image" again, I finally get a working page over Wi-Fi.

Unfortunately storing critical files on the filesystem and rebuilding it when uploading, means that building and flashing this project now depends on having tsc installed (eg. from npm), which is an added dependency. Without tsc, PlatformIO on Windows still uploads the partial filesystem image, but the file is effectively empty (compresses to about 1 kilobyte or so) since you don't (properly?) abort the build process on failure.
- IMO this makes it harder for users to build and setup a GBS-Control unit, unless we also offer prebuilt firmware images (in releases based on snapshots of known-good commits, and/or in CI which builds every push like #540).

(edit before posting) I just noticed that "separate build/upload/filesystem" is described in the altered README.md, including in a PRO Tip. It's quite unintuitive that "Build or Upload Filesystem Image" is necessary but not visible until after you build.

It's possible some of my objections to the build system ignore the ways it's more self-contained than the old one (IIRC requiring manually running shell scripts to build a web UI file from templates, then gzip and convert to a header). I think that prebuilt binaries would avoid the tsc issue (but not the "storing system files together with options/slots" issue).

Usage

Resolutions:

Weirdly the default preset says 240p in the slots list, even though the Wii is feeding 480p and the GBS-C is outputting 960p.
- (observation) You moved the output resolution picker from the first "presets" tab to the second "control" tab?
The web UI's Restart button does not work.
Your resolution options are misnamed and partially broken. The 240p button actually outputs 960p (but incorrectly shifted to the right). The 960p button does not switch resolutions, but reinitializes video output (screen goes blank and returns) while reusing the currently enabled scaling preset (eg. passthrough, 480p, or 960p).
If I switch to a fixed resolution on the second tab, then power-cycle the GBS-C, it reloads the currently selected custom slot. This differs from prior behavior where it would reload a fixed output resolution instead of a custom user-saved slot. Is the new behavior more intuitive to users? I suspect both are confusing?
If I select Pass Through, save a slot, and power-cycle the GBS-C, I get no image until I click 240p (produces a 960p output), then return to Pass Through.

Wi-Fi:

When the GBS-C is acting as an AP, and I close and reopen my M1 MacBook Air, I often cannot access the server at 192.168.4.1 until I disconnect and reconnect. I don't know if this error happens on master or not, and if it's a firmware bug or not.
- After trying to scan for access points and reconnecting, when trying to reload the web UI, I get an error like 7968 it's not enough memory.... The number changes when I reload. The error fixes after disconnecting and reconnecting to the AP.
- I have not tested for errors when using the GBS-C as a Wi-Fi client.
I'm getting FrameSyncManager::runFrequency() debug prints (when using clockgen-based frame sync), which I had hidden behind a commented-out debug define when writing this code. The messages clutter the log when you're looking for input signal change/loss issues, most users probably don't want to see them (and honestly even master probably prints "unnecessary" messages that confuses users, see #473).

There may be more things I've missed, but I wanted to get this post out reasonably soon.

way5 commented 5 months ago

In Arduino IDE, I upload with the NodeMCU v1.0 board configuration. On your branch, if I change platformio.ini to upload_resetmethod = nodemcu (tested on Windows) or remove the line (tested on Linux), I can upload to the board from PlatformIO.

Fixed, now it's uploading fine.

Building and uploading the firmware

After pulling your latest branch changes (bf8aebf, minus your latest commit), I can build on Windows now.

The last commit even more optimized for Windows

I didn't know the filesystem had to be uploaded separately from the firmware. I had assumed it was unchanged from master, where the web UI's source code is embedded as byte arrays into the firmware image, and the program is fully functional when the flash filesystem is cleared (files are written as you change settings or save presets).

Does this mean that users will have more steps to flash a working GBS-Control image?

if someone wish to make a clean install we can not forbid that on the contrary, we must make the process clear for everyone.

Will changes to the web UI's HTML file require flashing a new filesystem (I think containing file __index)? This erases saved presets and settings, which I don't think is a pleasant development experience (if I'm working on Web UI changes, I'd want to keep saved presets), though end users flash firmware less often.

Is there a specific reason you didn't leave index.html as part of the firmware image?

As I've mentioned before in terms of ease development I excluded WebUI data from image (temporary I believe). I don't have anything against to store it in flash while it's stored correctly.

To workaround this as a user, you can save all presets and reupload them after flashing a clean filesystem image. Maybe this is safer than "patching" the existing filesystem image with new system files (which could accumulate latent filesystem corruption like Race conditions, hanging, exceptions, and filesystem corruption when saving presets #421 and eventually break both saved presets and system files). It's still extra work that could be avoided by storing index.html in the firmware image directly.

Such things like #421 were not happening to me. May be because pio branch uses LittleFS already, as it is faster that SPIFFS. I can see the only threat when FS corruption may happen. It is when someone commits a faulty code, which actually may happen, we're all human. Anyways, there is BACKUP functionality working fine. It dumps all the contents of the flash, except system (__(.+)) files.

I'm concerned that storing system files on the filesystem instead of the firmware image makes them vulnerable to corruption during flash writes. It also allows the web UI to desynchronize with the underlying firmware, which can result in partial firmware updates (where the C++ code is uploaded but the web UI isn't) and broken systems if the communication protocol between the browser and firmware changes.

In my practice it happened only if I mess somewhere in the code. Right now I have running few ESP32s which are logging 24/7 onto its flash (with system files on flash). I am aware of flash wearing, but too long time should pass and too many read/writes must happen to flash till it fails. As I mentioned above, ...someone must commit a faulty code.

Unfortunately, on Windows "Upload Filesystem Image" generates an error and the GBS-C webserver sends the same 404 error as before.
WebUI ready to compress
'.' is not recognized as an internal or external command,
operable program or batch file.
*** Error 1
Building file system image from 'data' directory to .pio\build\d1_mini\littlefs.bin
/webui.html

Fixed.

Unfortunately storing critical files on the filesystem and rebuilding it when uploading, means that building and flashing this project now depends on having tsc installed (eg. from npm), which is an added dependency. Without tsc, PlatformIO on Windows still uploads the partial filesystem image, but the file is effectively empty (compresses to about 1 kilobyte or so) since you don't (properly?) abort the build process on failure.

I tgink for now the data directory could be removed from ignore to simplify the whole procedure for a user.

IMO this makes it harder for users to build and setup a GBS-Control unit, unless we also offer prebuilt firmware images (in releases based on snapshots of known-good commits, and/or in CI which builds every push like Add CI workflow with artifact creation for binary flashing #540). ... I think that prebuilt binaries would avoid the tsc issue (but not the "storing system files together with options/slots" issue).

That is why I had /builds directory there. The reason is to make it easy for everyone even without knowledge. CI is actually an objective. But I don't know when/if I actually make it :)

(edit before posting) I just noticed that "separate build/upload/filesystem" is described in the altered README.md, including in a PRO Tip. It's quite unintuitive that "Build or Upload Filesystem Image" is necessary but not visible until after you build.

I fixed some sentences there. That is mostly WIP areas that will be changing.

Usage

Resolutions:

Weirdly the default preset says 240p in the slots list, even though the Wii is feeding 480p and the GBS-C is outputting 960p.

This part requires testing and fine tuning. I could not yet do the comprehensive test on various devices.

(observation) You moved the output resolution picker from the first "presets" tab to the second "control" tab?

Correct, this is because resolutions will not be as a special presets, I believe users may wish to change resolutions independently of presets. The diagram that I've posted (in README) is actually about that. The old code has deep ambiguity whether you change slot+preset or switching resolutions+presets or a preset is set automatically and then user select a slot and then when he/she changes resolution everything falls apart. I believe that resolutions could be changed by user at any moment without changing other parameters, and to be stored as a parameter of each slot.

The web UI's Restart button does not work.

👍 Already fixed, goes with the next commit.

If I switch to a fixed resolution on the second tab, then power-cycle the GBS-C, it reloads the currently selected custom slot. This differs from prior behavior where it would reload a fixed output resolution instead of a custom user-saved slot. Is the new behavior more intuitive to users? I suspect both are confusing?

If I select Pass Through, save a slot, and power-cycle the GBS-C, I get no image until I click 240p (produces a 960p output), then return to Pass Through.

The system behavior while switching resolutions may change a bit in future, however "I believe that resolutions could be changed by user at any moment without changing other parameters, and to be stored as a parameter of each slot."

I'm getting FrameSyncManager::runFrequency() debug prints (when using clockgen-based frame sync), which I had hidden behind a commented-out debug define when writing this code. The messages clutter the log when you're looking for input signal change/loss issues, most users probably don't want to see them (and honestly even master probably prints "unnecessary" messages that confuses users, see remove "harmless" framesync frequency warnings, when debugging is turned off? #473).

Fixed.

TODO:

[x] > * When the GBS-C is acting as an AP, and I close and reopen my M1 MacBook Air, I often cannot access the server at 192.168.4.1 until I disconnect and reconnect. I don't know if this error happens on master or not, and if it's a firmware bug or not.

this may be mDNS or network specific issue or may be it's somehow related to M1 MacBook Air. I couldn't reproduce this case.
[x] > * After trying to scan for access points and reconnecting, when trying to reload the web UI, I get an error like 7968 it's not enough memory.... The number changes when I reload. The error fixes after disconnecting and reconnecting to the AP.

I could not reproduce this failure, however measures were taken to prevent possible service denials because of insufficient memory.
[ ] > * Your resolution options are misnamed and partially broken. The 240p button actually outputs 960p (but incorrectly shifted to the right). The 960p button does not switch resolutions, but reinitializes video output (screen goes blank and returns) while reusing the currently enabled scaling preset (eg. passthrough, 480p, or 960p).

That is something to check because it seems to be working fine to me.

way5 commented 5 months ago

@nyanpasu64, why was "matched presets" functionality added? The following is taken from original WebUI help message:

If enabled, default to 1280x960 for NTSC 60 and 1280x1024 for PAL 50 (does not apply for 720p / 1080p presets).

This statement says 1280x960@NTSC 60 and 1280x1024@PAL 50 will be forceable used except cases of 720p/1080p resolutions. Why is that? Is there an issue with output resolutions on some devices?

UPD: line number 4253 in master:

if (uopt->matchPresetSource && (result != 8) && (GBS::GBS_OPTION_SCALING_RGBHV::read() == 0)) {
    SerialM.println(F("matched preset override > 1280x960"));
    writeProgramArrayNew(ntsc_240p, false); // pref = x1024 override to x960
} else {
    writeProgramArrayNew(ntsc_1280x1024, false);
}

and then for PAL input (line 4269):

if (uopt->matchPresetSource) {
    SerialM.println(F("matched preset override > 1280x1024"));
    writeProgramArrayNew(pal_1280x1024, false); // pref = x960 override to x1024
} else {
    writeProgramArrayNew(pal_240p, false);
}

it is somewhat confusing logic, I believe there was a purpose to do so, but it is not clear to me.

Aleksandr-S12 commented 4 months ago

Не хочет собираться на Arduino 1.8.19 Выдает ошибку gbs-control\src\wifiman.cpp:48:9: note: in expansion of macro '_DBGF' _DBGF(PSTR("(WiFi): got IP: %s\n"), WiFi.localIP().toString()); Пожалуйста укажи версии библиотек. Пробовать каждый раз на угад, это полное безумие. Что касается кода, он так неудобно разбит на множество отдельных файлов. Хотел в твою версию добавить OSD, но разобраться, где что находиться практический невозможно, сразу отпала желание. Оригинальный код намного удобнее редактировать , там все важные функции в одном файле, а у тебя разбиты на сто файлов! Не визжу в этом смыла, создавать столько вкладок, сам же потом запутаешься. Могу быть не прав, так как не являюсь программистом

ramapcsx2 commented 4 months ago

I think the auto translator had tea time on this one ;p

Aleksandr-S12 commented 4 months ago

Hello ramapcsx2! This comment was left for Sergey Ko (way 5). My opinion is that what he did with the original GBS_C code is unacceptable. He split the main code into many tabs and renamed the variables, you don't need to do that! Complicated the procedure for installing firmware on the Arduino IDE platform, not everyone can use Visual Studio Code (PlatformIO). I will hope that the merge will not happen with the original GBS_C firmware. I didn't want to offend anyone here, that's my opinion from the outside!

ramapcsx2 commented 4 months ago

Wo gehobelt wird, fallen Späne. This is a major revamp of a bunch of code that really needs it. As long as it ultimately comes together, this is fine by me. (But it does have to come together ;p)

nyanpasu64 commented 4 months ago

Personally I think it's reasonable to keep this PR as a "fork" of the GBS-C codebase, and edit the README to point to the updated/changed fork as another choice. The advantage is that it would avoid breaking existing flashing/usage workflows (I don't fully understand the rewrite and last time I tested it changed the UI and had bugs in existing functionality) and I/rama/etc. can keep developing the old code and incorporating changes as needed. The disadvantage is that having multiple options for firmware may fragment development time and fixes, and confuse end users (until a consensus develops on which branch to officially continue going forward).

Aleksandr-S12 commented 4 months ago

Thank you, I understand everything. If a merge occurs, you will have to start work on embedding the on-screen menu again. I don't know for sure, but in the near future they may appear on Aliexpress GBS_C with an on-screen menu. Here is a photo of the GBS_C_OSD motherboard

Aleksandr-S12 commented 4 months ago

![Uploading MVI_8741.MP4_snap.680.jpg…]()

way5 commented 4 months ago

Приветствую!

В первую очередь, эта тема не для обсуждения проблем установки. Это запрос на слияние, что означает обсуждение вопросов по его подготовке и проведению, включая архитектуру продукта. В будущем; если у кого-то появилась проблема, милости прошу в issues.

Не хочет собираться на Arduino 1.8.19 Выдает ошибку gbs-control\src\wifiman.cpp:48:9: note: in expansion of macro '_DBGF' _DBGF(PSTR("(WiFi): got IP: %s\n"), WiFi.localIP().toString());

Указанная ошибка - есть нонсенс. Такое может произойти в случае если часть файлов была скопирована в ветку master вручную. В данной ветке (pio) с первых коммитов опубликован обновлённый README, где пошагово изложен процесс сборки (вместе с файловой системой) и установки. Ошибки при сборке случаются у тех, кто не читает инструкции. В данном случае инструкция = 6 простых шагов. Я озаботился и проверил, сделав установку всего начисто. То-есть удалил библиотеки Arduino IDE, клонировал последние версии в папку libraries в ТвоиДокументы/Arduino/:

git clone https://github.com/Links2004/arduinoWebSockets.git
git clone https://github.com/pavelmc/Si5351mcu.git
git clone https://github.com/ThingPulse/esp8266-oled-ssd1306.git

Затем, клонировал репозиторий, удалил main.cpp и собрал последний коммит без каких-либо проблем.
Отчёт о сборке приложен ниже.

capture

Пожалуйста укажи версии библиотек.

Самые актуальные версии. Больше нечего сообщить.

Пробовать каждый раз на угад, это полное безумие.

Наугад не надо. Следует потратить 3 минуты и ознакомиться с README

Что касается кода, он так неудобно разбит на множество отдельных файлов.

Подобное заявление кажется странным, особенно если ты не программист. Никто не запрещает использовать версию из оригинального master. Ветка pio создавалась с другой целью. So, why the functional approach, for instance in PHP, is better than MVC in 21st century? 😄

Хотел в твою версию добавить OSD, но разобраться, где что находиться практический невозможно, сразу отпала желание.

В "моей" версии OSD работает также, как и в мастере. Функционал OSD уже существует.

Оригинальный код намного удобнее редактировать , там все важные функции в одном файле, а у тебя разбиты на сто файлов! Не визжу в этом смыла, создавать столько вкладок, сам же потом запутаешься. Могу быть не прав, так как не являюсь программистом

"Дельный" совет от не-программиста? Not to be rude, again, you're free to use original master with all its heavy mess in code, obsolete sources, memory leaks and more if you wish. pio branch has been created for someone who wish to learn source code fast and have an easy and flawless install. You have failed both objectives... pity..

build.txt

way5 commented 4 months ago

Personally I think it's reasonable to keep this PR as a "fork" of the GBS-C codebase, and edit the README to point to the updated/changed fork as another choice.

Again, as I said earlier, I am perfectly OKAY with maintaining this branch myself on my repo as "yet another fork", IF everyone decides that. However in that case I'd prefer do not to be mentioned in

...the README to point to the updated/changed fork as another choice.

who knows what I'd implement there 😄, so you may feel uncomfortable to have links to a poor copy in the official repo.

The advantage is that it would avoid breaking existing flashing/usage workflows (I don't fully understand the rewrite and last time I tested it changed the UI and had bugs in existing functionality) and I/rama/etc. can keep developing the old code and incorporating changes as needed. The disadvantage is that having multiple options for firmware may fragment development time and fixes, and confuse end users (until a consensus develops on which branch to officially continue going forward).

I have to admit you're blazing fast😄 While someone is asking questions of the code, there is mostly stunning silence but everything changes when a "non-developer" reports of a non-existing problem. Rare ability to foster. 😄 There are also faithful followers in your team. 😁

Same with a dummy-locker commit shortly after this request was published. Initially I had in mind that local folks are interested in GBSC's evolution, but it seems the "to be or not to be" questions are more important nowadays.😄