IanSav wrote:...
I have some questions and observations regarding "Components/ServiceScan.py"...
What is the point, on lines 37 and 38, of locking the percentage complete to a maximum of 99%?
My guess is that it's to prevent it showing "100% completed" when it's still running. The text changes to "Scanning completed." when the scan has finished.
However, given the way that the percentage is calculated, it should never be more than 99 anyway.
IanSav wrote:s the "+" at the beginning of line 72 is valid? I have not seen this before and it looks wrong to me.
It depends on what you mean by "valid". It's part of a valid expression, provided tp.Modulation_16APSK is a type that is an acceptable target of a unary "+", which, since it's a C++ enum (with an integer base type), is true. Since it's unary '+', it will have no effect on the value, so the code will work as intended. My guess is that it's there as a result of a careless copy/paste from a unified (-u style) diff (e.g. from a git diff). In any case, it does look strange, it is redundant and it should probably go.
IanSav wrote:On line 68 the code "if tp_text == "DVB-S2":" causes the variable "tp_text" to be set but then on line 73 the value of "tp_text" is unconditionally changed making the previous definition irrelevant. Does the "+" from the previous point have any affect here? Is this another code problem?
No, it's not really making the test irrelevant. It's expanding the transponder information from just the name of the standard to the name of the standard followed by the modulation scheme. So "DVB-S2" might become, say, "DVB-S2 16APSK" or "DVB-S2 QAM16" if the test is true. I don't see anything broken here.
This code should never be reached in Beyonwiz T series devices.
IanSav wrote:The "getChannelNumber()" call on line 96 is not working. The function, defined in "Tools/Transponder.py", appears to be broken
There doesn't seem to be anything broken in Tools.Transponder.getChannelNumber(). Its implementation has changed, though, and the implementation requires the strings "DVB-T" and "Australia" to be in the "name" attribute of the <terrestrial/> element of terrestrial.xml for it to return channel numbers for Australian frequencies. These strings aren't currently in the "name" attribute. The "name" attribute would need to change from a string like "Sydney" to something like "Sydney Australia: DVB-T".
This is a problem with the current alpha. I'll post about the problem there and possibly make a new version of terrestrial.xml available for use with the alpha.
IanSav wrote:Wouldn't it be more efficient if most of this code was replaced by calls to "Tools/Transponder.py" to do most of the service identification / string conversion work. This looks like more copied code that increases support effort and the chances to introduce errors.
That's probably a good thing, provided the APIs are preserved, but there's a lot of this sort of stuff about. I hate to think about the number of different implementations of orbPos() (or orbpos()) there are lying around in the code.
NB: there's also an implementation of getChannelNumber() in Components.Converter.ChannelNumbers that should probably call Tools.Transponder.getChannelNumber(). It was that implementation that was previously used to get the channel number in Components.ServiceScan. "You are in a maze of twisty little passages, all alike."
IanSav wrote:This code would definitely benefit from a proper PEP clean up. ..
I can't disagree with that!
