Add hotspot vouchers

[ufozone]

@Kane610, I am unfortunately very new to Phyton / Home Assistant development and have no idea how to do add tests, add to the gateway class and so on.

@bmos has made some changes with regard to code hygiene. Thanks for that!

[Kane610]

@Kane610, I am unfortunately very new to Phyton / Home Assistant development and have no idea how to do add tests, add to the gateway class and so on.

@bmos has made some changes with regard to code hygiene. Thanks for that!

Here is a PR adding a new ApiHandler based class to the controller, adds api requests and tests, that should help you to get going

https://github.com/Kane610/aiounifi/commit/68ee89d27a154f609b95fc5c7bab126b52cda0e9

Don't hesitate to ask questions if you get stuck

[ufozone]

I have now built something for the PR from the linked PR. I didn't understand anything about the tests. I don't know if that's enough or how to test it.

[Kane610]

Skimmed it through, will review during the weekend

[ufozone]

@Kane610 , thank you very much. The suggested change has been implemented.

[ufozone]

@Kane610 , I have made adjustments.

I work on the tests blindfolded. If other things are missing here, I need support.

[Kane610]

@Kane610 , I have made adjustments.

I work on the tests blindfolded. If other things are missing here, I need support.

You're learning! :D

[ufozone]

These tests are still killing me. With small steps we are getting closer to the goal ... hopefully.

@Kane610 , take a look, I've done things again.

[Kane610]

If you can it's better if you can setup the environment locally so you can run the different tests, on linux it should only be to run the ./setup.sh and then source venv/bin/activate and you're running it

[ufozone]

Wow, thanks for your great support!

[ufozone]

✅ Voucher type definition properties and class methods adapted. ✅ Fix test fixture for voucher response. ✅ Minor bugs fixed and documentation improved

🤞Cross your fingers that the checks go through and we are ready for take off.

[Kane610]

Test fail, looks like you missed changing an id

[ufozone]

I first had to read up on what this coverage is. I'll give it a try.

[ufozone]

D'oh, now I see the mistakes too. 😪 I have eliminated them.

I prefer not to write how I develop... Spoiler: Not with a local runtime environment. 🤐

[Kane610]

You should change to do it locally

[ufozone]

woohoo 100% coverage.

You should change to do it locally

Oh yeah, I didn't realize how deep I was falling down the rabbit hole last year. I just wanted to write a little integration. Now there are three and we're in this PR here.

[Kane610]

I want ro go through it one last time. Jut I don't have time today and busy a few days so it would have to be on Monday

[ufozone]

So last commit

[ufozone]

Last attempt. Merge it or close this PR. It is difficult for me to document self-explanatory code in more detail.

[Kane610]

Last attempt. Merge it or close this PR. It is difficult for me to document self-explanatory code in more detail.

Maybe I'm using bad wording to express myself here and I apologize if that's the case. This should be read as me trying to motivate my issue with the current description in pydoc, what I have an understanding of what I'm missing in it based on your explanation and why this is important for me in the long term.

Look here. The comment says "Code in known format 00000-00000". If the code where to be self explanatory it would not do anything but making sure that there is a hyphen in the middle. Now the code does something else as well, which is not obvious to me either what that means. It should be obvious why these two paths exist. You had to play around with it yourself to figure out the five character limit.

I'm more or less asking you to rephrasing this "I experimented a bit and received vouchers with 5, 8 and regular 10 characters. When entering the code on the hotspot page, the hyphen must be placed after the fifth digit.". I don't think this is an unreasonable request worth getting defensive over.

I've previously accepted PRs with code that when I needed to change things had to spend a lot of time to understand to when refactoring things. If functionality is not clear then it will come back as a personal cost on me as I'm the one sitting on long term maintenance of the code base. That's why I want to make sure to make new mistakes and learn from old.

I hope I've explained myself enough and you have an understanding behind my motivations with this request.

/R

[ufozone]

In the Hotspot Manager UI, the regular 10-digit code is displayed with the hyphen "in the middle". However, the unofficial API delivers it without this hyphen.

Now it would be simple to deliver the code in the model as it comes from the API.

However, the captive portal expects the hyphen to be entered. So I build in the code the hyphen "in the middle" - f"{c[:5]}-{c[5:]}".

Through some nasty manipulation, I managed to get back a five and an eight-digit code. Putting a hyphen in there seems to be wrong. But maybe not. I don't know. Maybe it's just an edge case. There was no rocket science in the position of the hyphen.

That's why I've now simplified the code. If it cracks at some point, we'll work on it then.

[Kane610]

Test fails

FAILED tests/test_vouchers.py::test_vouchers[voucher_payload0] - AssertionError: assert '44703-' == '44703'

• 44703
• 44703-

Will the output from unifi be with or without hyphen?

[Kane610]

Right now the test expects "44703-"

[Kane610]

Suggested changes in https://github.com/Kane610/aiounifi/pull/554