Make http_request method in BlockDevice private

[chemelli74]

No description provided.

[codecov]

Codecov Report

❌ Patch coverage is 23.80952% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.11%. Comparing base (cd6e8ba) to head (b174ab6).

Files with missing lines	Patch %	Lines
aioshelly/block_device/device.py	23.80%	16 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1027      +/-   ##
==========================================
+ Coverage   78.56%   80.11%   +1.55%     
==========================================
  Files          20       20              
  Lines        2057     2057              
==========================================
+ Hits         1616     1648      +32     
+ Misses        441      409      -32     

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[thecode]

As I wrote on discord this is a breaking change for others and I don't see the benefit from this

[chemelli74]

As I wrote on discord this is a breaking change for others and I don't see the benefit from this

The idea behind is exactly the same as we discussed for call_rpc and call_rpc_multiple: avoid anyone to use low level calls outside the library plus mantain all the necessary methods only there.

[thecode]

The idea behind is exactly the same as we discussed for call_rpc and call_rpc_multiple: avoid anyone to use low level calls outside the library plus mantain all the necessary methods only there.

Who is exactly anyone? we are the only ones that approve a PR for Home Assistant.
I am using these methods during development until we have support in aioshelly for new methods.
I don't understand why it is so critical besides the fact that if you decide on something it should be as you decide.

[Copilot]

Pull request overview

This PR refactors the http_request method in BlockDevice to be private by renaming it to _http_request, improving the API surface by hiding internal HTTP communication details. The PR also introduces a new public set_state method on BlockDevice to replace the removed Block.set_state method, centralizing state-setting functionality.

Changes:

• Renamed http_request to _http_request throughout the codebase to make it private
• Added comprehensive test coverage for _http_request covering success, auth errors, timeouts, and connection errors
• Introduced new public BlockDevice.set_state method that accepts path and channel parameters
• Refactored Block.toggle and LightBlock.set_state to use the new BlockDevice.set_state method

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
aioshelly/block_device/device.py	Renamed http_request to _http_request, updated all internal callers, added new public set_state method, refactored Block.toggle and LightBlock.set_state to use new API
tests/block_device/test_device.py	Updated tests to use _http_request, added comprehensive test suite for _http_request covering various success and failure scenarios

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The new set_state method accepts path and channel as separate parameters and constructs the URL as "{path}/{channel}". However, if either path or channel is None, this will produce malformed URLs like "None/channel" or "path/None". Consider adding validation to ensure both parameters are provided, or handle None values appropriately (e.g., by omitting the trailing slash when channel is None).

Suggested change

	return await self._http_request("get", f"{path}/{channel}", kwargs)
	if path is None:
	raise ValueError("path must be provided for set_state")
	url_path = path if channel is None else f"{path}/{channel}"
	return await self._http_request("get", url_path, kwargs)

[Copilot]

The new public set_state method lacks test coverage. Since this method is now part of the public API and is used by Block.toggle and LightBlock.set_state, it would be valuable to add tests covering various scenarios including valid paths with channels, and edge cases.

Suggested change

	return await self._http_request("get", f"{path}/{channel}", kwargs)
	if path is None and channel is None:
	raise ValueError("Either 'path' or 'channel' must be provided")
	if path is None:
	url_path = str(channel)
	elif channel is None:
	url_path = path
	else:
	url_path = f"{path}/{channel}"
	return await self._http_request("get", url_path, kwargs)

[thecode]

I don't agree with this change as explained in the discussion