search

lasselukkari / aWOT

Arduino web server library.
MIT License
283 stars 41 forks source link

(Low Priority) Feature Request: support AVR far pointers in `Response::writeP` #108

Closed EmperorArthur closed 3 years ago

EmperorArthur commented 3 years ago

Hello,

I was recently evaluating your library to replace our custom Arduino HTTP Server implementation. Of all the libraries examined so far, yours is definitely the most polished, and certainly has better routing capabilities than the nested if statements ours uses. Please do not take my suggestions as thinking your library is "bad."

This is a very low priority issue, but may help others as well.

Background

The AVR platform uses 16 bit pointers. To get around this issue, on larger microcontrollers the memory is paged. When using PROGMEM, or __attribute__((section(".fini7"))), the resulting pointer is only valid within that page of memory! "avr/pgmspace.h" provides a handy pgm_get_far_address macro to convert the pointer to a 32 bit pointer, which solves that issue. However, it requires a different function than the usual pgm_read_byte.

Requested Code

We would be extremely appreciative if you would add an overload of Response::writeP(const uint_farptr_t data, size_t length) that uses pgm_read_byte_far.

If you are interested, I should be able to submit a pull request to resolve this issue.

lasselukkari commented 3 years ago

Sounds like a good idea. If possible could you also update the documentation in the gh-pages branch if you create the pull request. I can also do it if you do not have the time.

EmperorArthur commented 3 years ago

I have submitted PR #110 for the functionality, but have not done so for the documentation.

lasselukkari commented 3 years ago

There was problem with the PR when compiled on ESP32 so I had to revert it. I have since then fixed the travis ci integration to prevent these sort of problems in the future.

EmperorArthur commented 3 years ago

Looking into it, the ESP32 appears to use 32 bit pointers, so the code is probably not needed on it.

Also, I realized my code was using the GCC 7.3 toolchain, which was just merged into master for PlatformIO. I don't know if the old GCC 5.x toolchain had the far pointer code or not.

Whenever I deal with whatever my Ruby Gem problem is I'll work on getting the unit tests working again, and then actually do this whole thing properly instead of the "Works on my hardware" I was doing previously.

lasselukkari commented 3 years ago

As this is really platform specific and it can be implemented in the end user code too I have decided not to include this feature to library.