Skip to content
This repository has been archived by the owner on Dec 8, 2023. It is now read-only.

Use control schema for messages #49

Merged
merged 25 commits into from
May 10, 2020

Conversation

FloGa
Copy link
Contributor

@FloGa FloGa commented May 3, 2020

According to the specification for UPnP Devices, the arguments for the control actions need to match the order of arguments as desribed in the schema. To achieve that, we retrieve the schemas when searching for a compitable gateway and use the appropriate schema for each action to build the arguments properly.

Fixes #48.

FloGa added 19 commits May 2, 2020 17:00
If more parameters are introduced, the "if let" approach would involve
ever increasing levels of indentation. With dedicated matcher
statements, new parameters can be added without indentations.
The smaller functions make the code more readable and enable a more
functional syntax.
Since the new method will just return a Gateway with the exact same
parameters, we can also use the constructor directly, like in the
synchronous module.
The SearchState will be re-introduced with states related to the search
itself. The current "SearchState" is representing the state of the
Requests, hence the renaming.
The new way of controlling the search process allows to expand it with
additional stages.
According to the specification for [UPnP Devices][1], the arguments for
the control actions need to match the order of arguments as desribed in
the schema. To achieve that, we retrieve the schemas when searching for
a compitable gateway and use the appropriate schema for each action to
build the arguments properly.

[1]: https://openconnectivity.org/upnp-specs/UPnP-arch-DeviceArchitecture-v2.0-20200417.pdf
src/aio/search.rs Outdated Show resolved Hide resolved
src/aio/search.rs Outdated Show resolved Hide resolved
src/common/messages.rs Outdated Show resolved Hide resolved
FloGa added 3 commits May 4, 2020 19:31
This way we can use the logging macros in the whole library, not only in
aio.
To panic is not a nice way of handling unintended behavior of a library.
In case the router presents us with arguments we do not know about,
issue a warning via the logger, but do not bail out. Proceed as if
everything is okay and let the router decide whether it was fatal.
The concrete implementation of Future resulted in hard to read code,
with loops and pattern matching and passing status objects around.

Since we do not introduce new polling mechanisms here, but instead rely
on the implementations of hyper and tokio, it is sufficient to work with
async/.await instead of providing our own implementation of Future.

The resulting code is much shorter and easier to understand.
@FloGa
Copy link
Contributor Author

FloGa commented May 8, 2020

Hi Simon, I rewrote the aio.search module, this time without the Future implementation and just with async/.await.

src/aio/gateway.rs Outdated Show resolved Hide resolved
src/aio/gateway.rs Outdated Show resolved Hide resolved
@sbstp
Copy link
Owner

sbstp commented May 9, 2020

Looks good, I just have a few comments about the unwraps.

@FloGa
Copy link
Contributor Author

FloGa commented May 9, 2020

I rewrote the unchecked unwraps and replaced them with a proper Error variant for unsupported actions. Maybe this case will never occur, but who knows, what broken hardware / firmware is out there ...

@sbstp
Copy link
Owner

sbstp commented May 10, 2020

Looks! I'm gonna go a bit more testing and then merge. Are you ok with a squash merge?

@FloGa
Copy link
Contributor Author

FloGa commented May 10, 2020

Are you ok with a squash merge?

Personally, I don't like squash merges. It creates one huge commit, that is difficult to read and understand and it makes git bisect nearly impossible to use, should the need arise. I won't stop you from doing one though, but in my experience you will be happier with normal merges.

@sbstp
Copy link
Owner

sbstp commented May 10, 2020

I usually have a workflow that works well with squashes. I felt like I should ask here because you put a lot of care into making clean commits, so I won't squash it.

@FloGa
Copy link
Contributor Author

FloGa commented May 10, 2020

you put a lot of care into making clean commits

Thanks, it is exactly because of the reason I mentioned and it fits well in my workflow.

You should choose a method that fits with your usual workflow, I am fine with either way. :-)

@sbstp sbstp merged commit b96f590 into sbstp:master May 10, 2020
@sbstp
Copy link
Owner

sbstp commented May 10, 2020

Thanks, I've published 0.11.0 with this fix.

@FloGa
Copy link
Contributor Author

FloGa commented May 10, 2020

Great to hear, thank you very much!

@FloGa FloGa deleted the feature/use-control-schema-for-messages branch May 10, 2020 21:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wrong order of arguments in AddPortMapping
2 participants