Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: rework WoT discover method #133

Merged
merged 1 commit into from
May 19, 2024
Merged

feat!: rework WoT discover method #133

merged 1 commit into from
May 19, 2024

Conversation

JKRhb
Copy link
Member

@JKRhb JKRhb commented May 17, 2024

This PR will add a reworked version of the discover method to the project.

My plan is to be able to provide a global configuration for the method via the Servient class that could maybe even be overridden via a method parameter. However, in order to stay consistent with the specification, it might be better to allow configuration only by using the “underlying implementation”. I will experiment a bit in this PR with the different approaches before marking it as ready for review/discussion.

@JKRhb JKRhb force-pushed the discover-method branch 3 times, most recently from 906974a to 9541bb6 Compare May 18, 2024 23:50
@codecov-commenter
Copy link

codecov-commenter commented May 19, 2024

Codecov Report

Attention: Patch coverage is 1.14943% with 86 lines in your changes are missing coverage. Please review.

Project coverage is 66.80%. Comparing base (1a864a4) to head (36145b4).

Files Patch % Lines
lib/src/core/implementation/thing_discovery.dart 0.00% 76 Missing ⚠️
...lementation/discovery/discovery_configuration.dart 0.00% 9 Missing ⚠️
lib/src/core/implementation/wot.dart 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #133      +/-   ##
==========================================
+ Coverage   66.54%   66.80%   +0.25%     
==========================================
  Files          75       76       +1     
  Lines        2442     2434       -8     
==========================================
+ Hits         1625     1626       +1     
+ Misses        817      808       -9     

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

@JKRhb JKRhb force-pushed the discover-method branch from 8354f31 to c596e37 Compare May 19, 2024 16:35
@JKRhb JKRhb marked this pull request as ready for review May 19, 2024 16:35
@JKRhb
Copy link
Member Author

JKRhb commented May 19, 2024

I think this PR is now ready to be merged :) As indicated in the PR description, the discover method now uses the configuration of the Servient to perform discovery. The configuration is mutable can be adjusted at runtime, so that repeated calls with different configurations could be made.

It might make sense to also be able to override the configuration as a parameter in the discover method itself, but this might also require some more discussion in the Scripting API taskforce to determine if there could be a “well-known” set of discovery procedures that could be standardized. In any case, I think this PR can already serve as useful input for w3c/wot-scripting-api#535 and w3c/wot-scripting-api#551.

Following the merge of this PR, I will soon prepare a new minor release.

@JKRhb JKRhb merged commit b9f39b9 into main May 19, 2024
5 checks passed
@JKRhb JKRhb deleted the discover-method branch May 19, 2024 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants