Add curl option #13
Open
Add curl option #13
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 18 commits
August 4, 2024 22:28
Also change the DRIVER_MODIFICATION number even though I'm not sure if this should be done since this shouldn't change anything in the driver per se.
Add WITH_CURL compilation option. If compiled with WITH_CURL = YES, CurlUsage.template file will be added and it's records will be created. curl.h will be included. First record - UseCurl - should futurely toggle curl usage in the Driver.
No need to use getIntegerParam in this context if we already have the value argument.
Using this along with to-be-added CURLOPT_PASSWORD would expose credentials in the network. I leave it in the driver as a means to completely express the API functionalities, but there should be better ways of setting your credentials if you mean to. Probably will add a configuration file for that in the future.
Same warning about last commit: this shouldn't be used but it's left here as an option for quick testing and better expressing the API functionalities. For actual credentials usage future records should add a path for a configuration file containing the credentials.
Should be used to load the file contents and put them into the correct parameters. Uses a very similar mechanism from NDFile.template and functions from asynNDArrayDriver. For now it just checks if the file exists and is accessible, but not it's contents.
Add CurlLoadConfig record and loadConfigFile function. loadConfigFile() calls writeInt32 and writeOctet for all parameters it can recognize, except username and password. For those, loadConfigFile() calls curl_easy_setopt without ever setting the parameter value. This means setpoint and RBV values for all parameters can be different, so I add readback values for all records. writeInt32 and writeOctet are called inside loadConfigFile(), which is called inside writeInt32. In my understanding this cannot lead to any infinite loop since CurlLoadConfig record has no "ASYN_" prepending it's name, so it's impossible for it to call writeInt32 on itself. Also, I think this is thread safe because the function is only supposed to be called inside writeInt32, which is supposed to be already locked.
Readbuffer is a char vector just so we can copy all needed bytes to it. writeCallback should simply add stuff to the buffer. readImage clears buffer before adding to it. Unfortunately I think clear cant be called inside writeCallback because it's called more than one time per read.
This is obviously needed and acquisition will fail without this.
WITH_CURL = YES/NO is used to tell the compiler to use or not use curl. I add an example in configure/CONFIG_SITE to make it easy for people to enable/disable this in future.
Curl is added as an option both at compile and at runtime. If compiled with WITH_CURL = YES at configure/CONFIG_SITE.local, curl option will be enabled at runtime but default will still be UseCurl = NO. If record $(P)$(R)UseCurl = YES, then code will get image from URL using libCurl instead of graphicsMagick. Some curlOptions are configurable as records. More can be easily added. The CurlConfigFile.template is based on NDFile.template, but a bit different, so I preferred to create a new one instead of including the one that already exists. Maybe this can be modified in future.
|
I have never used .rst files before so will take some time looking at how to put this in the documentation. Any hints are welcome :) |
|
Almost all of the other areaDetector drivers and plugins have .rst documentation files that you can use as examples. |
added 5 commits
August 22, 2024 16:05
Describe all new records along with brief driver description.
Comments to make it easier to load Curl template file if compiled with WITH_CURL=YES
Add records and driver description into ADURL.rst and code comments in each function.
Closed
|
Thanks for the suggestions! I added Driver and records updated description to docs/ADURL/ADURL.rst file and code comment. Is there any other documentation that should be updated in this PR? |
RELEASE.md should be updated with some notes about this addition. |
Since this PR is supposed to add new functionality, I reset patch number and add to revision number.
added 3 commits
August 23, 2024 14:08
Update with info about header file change and curl functionality added.
Post merge of doc branch into add-curl-option.
|
Thanks, done in commit 74aa1a6 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds Curl as an option to get image from URL. Option can be set both at compilation time (as suggested by @EmilioPeJu) and runtime.
When compiled with WITH_CURL = YES, records such as '$(P)$(R)UseCurl' can be used, but the default is still '$(P)$(R)UseCurl = NO'. If compiled with WITH_CURL different than YES, code is basically the same as before this pull request.
When'$(P)$(R)UseCurl = 1', Image is retrieved from URL with curl_easy_perform() function. Curl parameters can be defined via records and all implemented records can be loaded from a configuration file.
Particularly CURLOPT_PASSWORD and CURLOPT_USERNAME are defined as curl options but - if defined via configuration file - are not passed to asyn parameters so you will neither broadcast your credentials in the network nor let someone see it with asynReport.
I chose to use a custom file loader instead of toml.h because toml.h would increase compilation time substantially.
Also, not all curl options are implemented as it would take lots of time to implement all and they can be easily implemented as needed, if and when needed.
Tested with Single, Multiple and Continuous acquisition modes, with Hikivision camera and also a few images from google images that I couldn't manage to take with regular graphicsmagick stuff such as Kawaii Cthulhu: https://i.imgflip.com/2yp8up.jpg
Example acquisition test:
Where myconfig contains, just as an example: