8000
Skip to content

[online_image] Set Accept header - #8216

Merged
jesserockz merged 3 commits into
esphome:devfrom
candrews:online_image_headers
Feb 10, 2025
Merged

[online_image] Set Accept header#8216
jesserockz merged 3 commits into
esphome:devfrom
candrews:online_image_headers

Conversation

@candrews
@candrews candrews commented Feb 6, 2025
Copy link
Copy Markdown
Contributor

What does this implement/fix?

The HTTP Accept request header indicates which content types, expressed as MIME types, the sender is able to understand. In requests, the server uses content negotiation to select one of the proposals.

See: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Accept

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Code quality improvements to existing code or addition of tests
  • Other

Related issue or feature (if applicable):

n/a

Pull request in esphome-docs with documentation (if applicable):

n/a

Test Environment

  • ESP32
  • ESP32 IDF
  • ESP8266
  • RP2040
  • BK72xx
  • RTL87xx

Example entry for config.yaml:

Any example would work, such as the one shown in https://esphome.io/components/online_image.html

online_image:
  - url: "https://example.com/example.png"
    format: png
    id: my_online_image

Checklist:

  • The code change is tested and works locally.
  • Tests have been added to verify that the new code works (under tests/ folder).

If user exposed functionality or configuration variables are added/changed:

The HTTP Accept request header indicates which content types, expressed as MIME types, the sender is able to understand. In requests, the server uses content negotiation to select one of the proposals.

See: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Accept
@probot-esphome
probot-esphome Bot commented Feb 6, 2025
Copy link
Copy Markdown

Hey there @clydebarrow, @guillempages, mind taking a look at this pull request as it has been labeled with an integration (online_image) you are listed as a code owner for? Thanks!
(message by CodeOwnersMention)

@codecov-commenter
codecov-commenter commented Feb 6, 2025
Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 53.81%. Comparing base (4d8b5ed) to head (dd71b15).
Report is 1984 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #8216      +/-   ##
==========================================
+ Coverage   53.70%   53.81%   +0.10%     
==========================================
8000

  Files          50       50              
  Lines        9408     9856     +448     
  Branches     1654     1360     -294     
==========================================
+ Hits         5053     5304     +251     
- Misses       4056     4230     +174     
- Partials      299      322      +23     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@guillempages guillempages left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you tested this with the arduino framework as well? AFAIK, the http_request implementation is very different for both esp_idf and arduino.

Comment thread esphome/components/online_image/online_image.cpp
Comment thread esphome/components/online_image/online_image.cpp Outdated
Comment thread esphome/components/online_image/online_image.cpp Outdated
@guillempages
Copy link
Copy Markdown
Contributor

Is there a use case that you have encountered for this? If the accept is still falling back to accepting everything anyway, and the user already needs to know which format the downloaded image is in, what's the point of adding the accept header at all? In which way does this help? (Just trying to understand; not trying to reject the change)

@candrews
candrews commented Feb 6, 2025
Copy link
Copy Markdown
Contributor Author

Is there a use case that you have encountered for this? If the accept is still falling back to accepting everything anyway, and the user already needs to know which format the downloaded image is in, what's the point of adding the accept header at all? In which way does this help? (Just trying to understand; not trying to reject the change)

Servers use the accept header to respond with the correctly formatted image. For example, I'm generating webp or png based on the Accept header in my image generating service.

By having online_image send the appropriate Accept header, when online_image gains support for a new format, the server will automatically return the image in that new format.

8000

@guillempages
Copy link
Copy Markdown
Contributor

Ok, I wouldn't have expected that for the esphome devices this is actually a use case. I would have expected people to just download existing images, or have a server explicitly generating the image for ESPHome, so there would be no need for the accept headers.

Nevertheless, this is an improvement so I have no objection to getting this in.

@jesserockz
jesserockz merged commit 45b8810 into esphome:dev Feb 10, 2025
@github-actions github-actions Bot locked and limited conversation to collaborators Feb 12, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

0