kellerza / pysma

Async library for SMA Solar's WebConnect interface
MIT License
59 stars 51 forks source link

Remove output of sma_sid, because it doesn't always exist #77

Closed msarahan closed 3 years ago

msarahan commented 3 years ago

I have a SunnyBoy 5.0, and this particular attribute doesn't exist. It is _sid instead for me. Since the traceback isn't all that great for newbies approaching the project, I think it might be better to just leave this part out.

Traceback, for reference:

Traceback (most recent call last):
  File "/home/msarahan/code/monitoring/sensors/example.py", line 104, in <module>
    main()
  File "/home/msarahan/code/monitoring/sensors/example.py", line 98, in main
    loop.run_until_complete(
  File "/home/msarahan/.local/share/r-miniconda/envs/sma/lib/python3.9/asyncio/base_events.py", line 642, in run_until_complete
    return future.result()
  File "/home/msarahan/code/monitoring/sensors/example.py", line 50, in main_loop
    _LOGGER.info("NEW SID: %s", VAR["sma"].sma_sid)
AttributeError: 'SMA' object has no attribute 'sma_sid'
codecov[bot] commented 3 years ago

Codecov Report

Merging #77 (a639410) into master (99b6c7c) will not change coverage. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master       #77   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            6         6           
  Lines          403       404    +1     
=========================================
+ Hits           403       404    +1     
Impacted Files Coverage Δ
pysma/__init__.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 99b6c7c...a639410. Read the comment docs.

msarahan commented 3 years ago

I'm not sure that's a great example?

Make attributes private so they dont show in the docs. Direct access should not be required.

Then why show direct access in the example?

I'm happy to change this so that it works, I just want the example to be as friendly and useful as possible.

rklomp commented 3 years ago

Yeah you are right. We could remove this and add a logger.debug statement in de new_session method of SMA to replace it.

msarahan commented 3 years ago

nice, I think that's a good place for it. Thanks!