Desktop support (Linux / Windows / Mac) (ESF-122) #102

electromuis · 2024-03-22T11:25:51Z

For a board I made using the esp32s3 I also have a desktop client written in c++.

It would be great if users could update the firmware directly from the tool. Wanting to keep the tool small by not including a python install to use esptool, I found this library to contain the most important parts.

I've done some work to make it work on x86 systems by using the https://github.com/wjwwood/serial library.

It's not completed yet and could probably use a good eye but I wanted to share it at least so other interested people could find it.

A new example is added called: desktop_esp32_example. It builds and rust on my windows system. Have not tried flashing to an actual board yet, will have to check got esptool does the resetting since normally a PC wouldn't have gpio access.

electromuis · 2024-03-22T14:29:27Z

Update: firmware flashing works

DNedic · 2024-03-22T14:59:14Z

examples/common/example_common.c

@@ -336,7 +346,8 @@ esp_loader_error_t load_ram_binary(const uint8_t *bin)
    printf("Start loading\n");
    esp_loader_error_t err;
    const esp_loader_bin_header_t *header = (const esp_loader_bin_header_t *)bin;
-    esp_loader_bin_segment_t segments[header->segments];
+    //esp_loader_bin_segment_t segments[header->segments];


Why is this being moved to the heap?

Because it didn't compile on MSVC

Ah yes, after C99 VLAs are a GNU extension as well. We shouldn't be using VLAs in the first place IMO but I am unsure about using malloc(), as we might want to support platforms in the future that don't have it available. Also, using malloc() in example code for embedded platforms where malloc() is often prohibited is not ideal, especially as some of these example functions happen to be used in production a lot of the time.

A simple static array with a max segment count should suffice.

private_include/protocol.h

DNedic · 2024-03-22T15:00:22Z

src/protocol_common.c

@@ -295,7 +295,7 @@ esp_loader_error_t loader_spi_parameters(uint32_t total_size)
    return send_cmd(&spi_cmd, sizeof(spi_cmd), NULL);
 }

-__attribute__ ((weak)) void loader_port_debug_print(const char *str)


Probably leftover code from testing, a weak symbol shouldn't hurt anything.

@DNedic I think the reason is that __attribute__((weak)) syntax is a GNU extension, and is not supported by MSVC compiler. Same for packed — MSCV has a different way of specifying the same.

I think this will still need to be cleaned up to move all the compiler-specific things into a common header.

I agree, we should move these platform details into a separate header if we merge this.

Since LLVM GNU extensions, is there a way to build for Windows using LLVM and just not bother with MSVC? As far as I've seen the incompatibilities go deeper than GNU extensions.

Mingw might work, but I'd prefer an "if msvc else" precompiler flag. Is does actually compile but the problem is this symbol name is being used multiple times and thus the linker complains. Renaming one of them would also fix the issue.

port/serial_port.h

DNedic · 2024-03-22T15:05:54Z

port/serial_port.h

+} loader_serial_config_t;
+
+esp_loader_error_t loader_port_serial_init(const loader_serial_config_t *config);
+


Perhaps a deinit function would be useful?

igrr · 2024-03-22T15:06:16Z

.gitmodules

@@ -1,3 +1,6 @@
 [submodule "submodules/stm32-cmake"]
 	path = submodules/stm32-cmake
 	url = https://github.com/ObKo/stm32-cmake.git
+[submodule "submodules/serial"]
+	path = submodules/serial
+	url = https://github.com/wjwwood/serial


Just my 2c, I would recommend fetching this project at build time using ExternalProject or FetchContent CMake modules, instead of adding a submodule into esp-serial-flasher.

Good idea! This would be a good time to move over the stm32-cmake submodule to FetchContent as well.

Perhaps a find_package script which can do the fetching if needed. In my application the serial library is already present. As a note, I didn't use add_subdirectory on this since it wants catkin as a dependancy, which I don't need here.

port/serial_port.cpp

DNedic · 2024-03-22T15:10:09Z

Hello @electromuis , nice work and thank you for the contribution! I've left some initial comments, most of them are probably due to this being WIP.

electromuis · 2024-03-22T15:37:01Z

Hello @electromuis , nice work and thank you for the contribution! I've left some initial comments, most of them are probably due to this being WIP.

Thanks for the comments, added my replies. The README could also be updated since "from other host microcontrollers" doesn't count for a big desktop CPU haha

electromuis · 2024-03-28T13:55:25Z

Tried to address each feedback point

DNedic · 2024-03-28T14:08:08Z

README.md

@@ -1,8 +1,10 @@
 # esp-serial-flasher

-`esp-serial-flasher` is a portable C library for flashing or loading apps to RAM of Espressif SoCs from other host microcontrollers.
+`esp-serial-flasher` is a portable C library for flashing or loading apps to RAM of Espressif SoCs from other host that are supported through their dedicated port drivers.


Perhaps it's better to be upfront here. I would preffer something like:

`esp-serial-flasher` is a portable C library for flashing or loading apps to RAM of Espressif SoCs from other microcontrollers, SBCs and even desktop PCs.

DNedic · 2024-03-28T14:09:40Z

port/wjwwood_serial_port.cpp

+        return;
+    }
+
+    // This might be the best we can do without GPIO


Please take a look at how esptool does this. There are several reset strategies depending on whether the host is Windows/Linux or MacOS.

The wjwwood serial library seems to have support for setting DTR/RTS
https://github.com/wjwwood/serial/blob/69e0372cf0d3796e84ce9a09aff1d74496f68720/src/serial.cc#L404-L406

Questions is whether setting them independently is fast enough. From my experience: most likely not.

I've used the serial class from Qt which has similar abstractions and still had to resort to OS specific calls to switch DTR/RTS at the same time.

Isn't that what @electromuis is doing here? The question I was addressing is whether the platform specifics have propagated to the library itself. If not, this code is good as is.

I think what he means is dtr and rts would need to switch in a single call instead of the two -> set calls listed currently.

The board I'm using currently doesn't even reset with esptool.py so hard to test

I think what he means is dtr and rts would need to switch in a single call instead of the two -> set calls listed currently.

The board I'm using currently doesn't even reset with esptool.py so hard to test

Yes that's what I've ment.

electromuis added 2 commits March 22, 2024 12:15

Working on desktop support

1f6ca72

Merge branch 'master' of https://github.com/espressif/esp-serial-flasher

947301c

github-actions bot changed the title ~~Desktop support (Linux / Windows / Mac)~~ Desktop support (Linux / Windows / Mac) (ESF-122) Mar 22, 2024

Serial port config update. Firmware flash working

98d215b

DNedic reviewed Mar 22, 2024

View reviewed changes

private_include/protocol.h Show resolved Hide resolved

DNedic reviewed Mar 22, 2024

View reviewed changes

port/serial_port.h Outdated Show resolved Hide resolved

DNedic reviewed Mar 22, 2024

View reviewed changes

igrr reviewed Mar 22, 2024

View reviewed changes

DNedic reviewed Mar 22, 2024

View reviewed changes

port/serial_port.cpp Outdated Show resolved Hide resolved

Fixed timing bug. Added PR feedback

4b650f8

DNedic reviewed Mar 28, 2024

View reviewed changes

DNedic added the enhancement label May 21, 2024

tomassebestik removed the enhancement label Sep 22, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Desktop support (Linux / Windows / Mac) (ESF-122) #102

Desktop support (Linux / Windows / Mac) (ESF-122) #102

electromuis commented Mar 22, 2024

electromuis commented Mar 22, 2024

DNedic Mar 22, 2024

electromuis Mar 22, 2024

DNedic Mar 22, 2024 •

edited

Loading

DNedic Mar 22, 2024

DNedic Mar 22, 2024

igrr Mar 22, 2024

DNedic Mar 22, 2024 •

edited

Loading

DNedic Mar 22, 2024 •

edited

Loading

electromuis Mar 22, 2024

DNedic Mar 22, 2024 •

edited

Loading

electromuis Mar 22, 2024

igrr Mar 22, 2024

DNedic Mar 22, 2024

electromuis Mar 22, 2024

DNedic commented Mar 22, 2024 •

edited

Loading

electromuis commented Mar 22, 2024

electromuis commented Mar 28, 2024

DNedic Mar 28, 2024

DNedic Mar 28, 2024

higaski Mar 28, 2024

DNedic Mar 28, 2024

electromuis Mar 28, 2024

higaski Mar 28, 2024

		} loader_serial_config_t;

		esp_loader_error_t loader_port_serial_init(const loader_serial_config_t *config);

Desktop support (Linux / Windows / Mac) (ESF-122) #102

Are you sure you want to change the base?

Desktop support (Linux / Windows / Mac) (ESF-122) #102

Conversation

electromuis commented Mar 22, 2024

electromuis commented Mar 22, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DNedic Mar 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DNedic Mar 22, 2024 • edited Loading

Choose a reason for hiding this comment

DNedic Mar 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DNedic Mar 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DNedic commented Mar 22, 2024 • edited Loading

electromuis commented Mar 22, 2024

electromuis commented Mar 28, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DNedic Mar 22, 2024 •

edited

Loading

DNedic Mar 22, 2024 •

edited

Loading

DNedic Mar 22, 2024 •

edited

Loading

DNedic Mar 22, 2024 •

edited

Loading

DNedic commented Mar 22, 2024 •

edited

Loading