Re: [melbourne-pug] FroSolPy Fronius Inverter Data Collector / Code Feedback

[David Crisp]

Hi Dave,
Thank you for these ideas.Since I posted this the code has got longer
as I have dived in and added more comments and docstrings to all the
properties.   Once I have finished commenting and documenting the
functional code I will look at breaking it out in to separate files.   I
could probably do it BEFORE I finish the commenting but i'm on a bit of a
role and if I get distracted I will probably never get back to it.
Regards,
David

On 18 May 2018 at 14:41, David O'Keeffe  wrote:

> Hey David,
>
> I've been working on something similar for the Sungrow SH5K inverter
> talking through the Modbus protocol. From looking over your code briefly,
> it's an almost 2000 line script, I can't easily make sense of what your
> logic is.
>
> I'd take the long list of initializations out as a dictionary in another
> py file and write an abstract function for code blocks like this.
>
> @property
> def PowerApparent_S_Phase_1(self):
> if (self._checkdatacurrency(self.MeterRealTimeData.
> PowerApparent_S_Phase_1)):
> return self.MeterRealTimeData.PowerApparent_S_Phase_1.Value
> else:
> self._GetMeterRealtimeData()
> return self.MeterRealTimeData.PowerApparent_S_Phase_1.Value
> @property
> def PowerApparent_S_Phase_2(self):
> if (self._checkdatacurrency(self.MeterRealTimeData.
> PowerApparent_S_Phase_2)):
> return self.MeterRealTimeData.PowerApparent_S_Phase_2.Value
> else:
> self._GetMeterRealtimeData()
> return self.MeterRealTimeData.PowerApparent_S_Phase_2.Value
> Cheers,
> Dave
>
>
> On Fri, May 18, 2018 at 2:05 PM, William ML Leslie <
> william.leslie@gmail.com> wrote:
>
>> On 18 May 2018 at 13:40, paul sorenson  wrote:
>> > My inverter came with a CD-ROM which would push a cloud somewhere but I
>> > reckon it would be fun to crowd source really granular data.
>> >
>>
>> The ability to push clouds is a great feature for a solar inverter to
>> have.
>>
>> --
>> William Leslie
>>
>> Notice:
>> Likely much of this email is, by the nature of copyright, covered
>> under copyright law.  You absolutely MAY reproduce any part of it in
>> accordance with the copyright law of the nation you are reading this
>> in.  Any attempt to DENY YOU THOSE RIGHTS would be illegal without
>> prior contractual agreement.
>> ___
>> melbourne-pug mailing list
>> melbourne-pug@python.org
>> https://mail.python.org/mailman/listinfo/melbourne-pug
>>
>
>
> ___
> melbourne-pug mailing list
> melbourne-pug@python.org
> https://mail.python.org/mailman/listinfo/melbourne-pug
>
>
___
melbourne-pug mailing list
melbourne-pug@python.org
https://mail.python.org/mailman/listinfo/melbourne-pug

Re: [melbourne-pug] FroSolPy Fronius Inverter Data Collector / Code Feedback

[David Crisp]

Hello Alaa,

First off I must apologise for not responding earlier,   gmail insists on
filing all my melbourne-pug emails way over there hidden away from my
normal inbox!

Thank you for looking at the code and providing feedback! This is the sort
of useful information I was hoping to get!   Once I have had a chance to
process the information I will comment on why i did things originally.

Regards,
David

On 18 May 2018 at 23:33, Alaa Salman  wrote:

> Hi David,
>
> I am not sure what kind of code review you're after but I had a quick look
> at the code and though I can't comment on the functionality, there are a
> few things that stand out.
>
> 1- That's a lot of code to sit in a single file, you might want to
> consider splitting it up over multiple files or modules
>
> 2- The code pattern where you check for key existence before you fetch its
> value can be replaced with the get() dict function which would cut down on
> a lot of the code. Some lines in there can also be replaced with a default
> dict.
>
> 3- You'll find that most python code follows the directions in pep8 for
> style. There's no right or wrong for this one just convention/consistency
> and tradition.
>
> 4- I am not sure why you're doing __new__.__defaults__ = (None,) * len(
> self.CommonInverterValues.IAC._fields). There are more expressive ways to
> do this.
>
> 5- Something like "url={protocol}://{host}" can be extracted to a
> common location since usually this wouldn't change for a single invocation.
>
> 6- Using named tuples inside of classes might make your code a bit more
> difficult to follow. Maybe consider encapsulating the different objects in
> different classes and populate those.
>
> 7- You mentioned there are no tests. I don't see any logic in there that
> could use testing. However, you can write tests to make sure that your code
> handles unexpected values as a start.
>
>
> I hope this helps. Very nice work on commenting your code and keeping it
> tidy. Please feel free to ask any questions you might have to the list and
> I'm sure we're all happy to help.
>
>
>
> On 18/05/18 14:05, melbourne-pug-requ...@python.org wrote:
>
> --
>
> Message: 1
> Date: Fri, 18 May 2018 09:35:16 +1000
> From: David Crisp  
> To: Melbourne Python Users Group  
> 
> Subject: [melbourne-pug] FroSolPy Fronius Inverter Data Collector /
>   Code Feedback
> Message-ID:
>
> 
> Content-Type: text/plain; charset="utf-8"
>
> Gday,
>
> I'm not sure if this is appropriate to ask for or not but I was wondering
> if there was anybody who would be happy to do a quick code review / code
> feedback on my Fronius Solar module I have written  and give me some
> feedback on it.
>
> I have been working on this module for a while and I think I'm beginning to
> not be able to see the trees for the forest.   It is NOT finished yet but
> it does what I need it to do for the moment.
>
> There's no unit tests though.  I haven't worked out how to do these for
> dynamic data collected from APIs etc which could return anything.
>
> Currently being unemployed and not having access to a development team I
> don't get a chance to drop code in front of more experienced people and get
> ideas from them.
>
> The module should be able to be found at the following 
> location.https://github.com/dcrispgit/FroSolPy
>
> Bonus Points if you have your own Fronius solar inverter and you can
> actually run this code and retrieve data from it.
>
> If it's not appropriate to ask that then feel free to ignore or point me in
> the direction of somewhere that can help.
>
> Regards,
> David
> -- next part --
> An HTML attachment was scrubbed...
> URL: 
> 
>  
> 
>
> --
>
>
>
> ___
> melbourne-pug mailing list
> melbourne-pug@python.org
> https://mail.python.org/mailman/listinfo/melbourne-pug
>
>
___
melbourne-pug mailing list
melbourne-pug@python.org
https://mail.python.org/mailman/listinfo/melbourne-pug

Re: [melbourne-pug] FroSolPy Fronius Inverter Data Collector / Code Feedback

[David O'Keeffe]

Hey David,

I've been working on something similar for the Sungrow SH5K inverter
talking through the Modbus protocol. From looking over your code briefly,
it's an almost 2000 line script, I can't easily make sense of what your
logic is.

I'd take the long list of initializations out as a dictionary in another py
file and write an abstract function for code blocks like this.

@property
def PowerApparent_S_Phase_1(self):
if (self._checkdatacurrency(self
.MeterRealTimeData.PowerApparent_S_Phase_1)):
return self.MeterRealTimeData.PowerApparent_S_Phase_1.Value
else:
self._GetMeterRealtimeData()
return self.MeterRealTimeData.PowerApparent_S_Phase_1.Value
@property
def PowerApparent_S_Phase_2(self):
if (self._checkdatacurrency(self
.MeterRealTimeData.PowerApparent_S_Phase_2)):
return self.MeterRealTimeData.PowerApparent_S_Phase_2.Value
else:
self._GetMeterRealtimeData()
return self.MeterRealTimeData.PowerApparent_S_Phase_2.Value
Cheers,
Dave


On Fri, May 18, 2018 at 2:05 PM, William ML Leslie <
william.leslie@gmail.com> wrote:

> On 18 May 2018 at 13:40, paul sorenson  wrote:
> > My inverter came with a CD-ROM which would push a cloud somewhere but I
> > reckon it would be fun to crowd source really granular data.
> >
>
> The ability to push clouds is a great feature for a solar inverter to have.
>
> --
> William Leslie
>
> Notice:
> Likely much of this email is, by the nature of copyright, covered
> under copyright law.  You absolutely MAY reproduce any part of it in
> accordance with the copyright law of the nation you are reading this
> in.  Any attempt to DENY YOU THOSE RIGHTS would be illegal without
> prior contractual agreement.
> ___
> melbourne-pug mailing list
> melbourne-pug@python.org
> https://mail.python.org/mailman/listinfo/melbourne-pug
>
___
melbourne-pug mailing list
melbourne-pug@python.org
https://mail.python.org/mailman/listinfo/melbourne-pug

Re: [melbourne-pug] FroSolPy Fronius Inverter Data Collector / Code Feedback

[William ML Leslie]

On 18 May 2018 at 13:40, paul sorenson  wrote:
> My inverter came with a CD-ROM which would push a cloud somewhere but I
> reckon it would be fun to crowd source really granular data.
>

The ability to push clouds is a great feature for a solar inverter to have.

-- 
William Leslie

Notice:
Likely much of this email is, by the nature of copyright, covered
under copyright law.  You absolutely MAY reproduce any part of it in
accordance with the copyright law of the nation you are reading this
in.  Any attempt to DENY YOU THOSE RIGHTS would be illegal without
prior contractual agreement.
___
melbourne-pug mailing list
melbourne-pug@python.org
https://mail.python.org/mailman/listinfo/melbourne-pug

Re: [melbourne-pug] FroSolPy Fronius Inverter Data Collector / Code Feedback

[paul sorenson]

Hey David,

I probably can't run that on my inverter but I wrote some similar code
for Aurora brand a while back.

My inverter came with a CD-ROM which would push a cloud somewhere but I
reckon it would be fun to crowd source really granular data.

cheers


On 05/17/2018 04:35 PM, David Crisp wrote:
> Gday,
>
> I'm not sure if this is appropriate to ask for or not but I was
> wondering if there was anybody who would be happy to do a quick code
> review / code feedback on my Fronius Solar module I have written  and
> give me some feedback on it.  
>
> I have been working on this module for a while and I think I'm
> beginning to not be able to see the trees for the forest.   It is NOT
> finished yet but it does what I need it to do for the moment.
>
> There's no unit tests though.  I haven't worked out how to do these
> for dynamic data collected from APIs etc which could return anything.
>
> Currently being unemployed and not having access to a development team
> I don't get a chance to drop code in front of more experienced people
> and get ideas from them.
>
> The module should be able to be found at the following location.
> https://github.com/dcrispgit/FroSolPy
>
> Bonus Points if you have your own Fronius solar inverter and you can
> actually run this code and retrieve data from it.
>
> If it's not appropriate to ask that then feel free to ignore or point
> me in the direction of somewhere that can help.
>
> Regards, 
> David
>
>
> ___
> melbourne-pug mailing list
> melbourne-pug@python.org
> https://mail.python.org/mailman/listinfo/melbourne-pug

___
melbourne-pug mailing list
melbourne-pug@python.org
https://mail.python.org/mailman/listinfo/melbourne-pug