108 replies to this topic

[Huevos]

And the stupidest thing of all is using configFloat for something that is not a float at all.

 

Convert an int to configFloat and then convert configFloat back to an int. Absolutely crazy.

[Huevos]

All this extra complexity and it is still just a six figure number field in the UI.

Edited by Huevos, 11 October 2016 - 17:29.

[littlesat]

I'm afraid this is because you patched a bit too much in the default cable transponder... you removed three zero's
It is better to wait... ConfigFloat class could be changed a bit....
And be honest... I never saw another ui or list where these fre us cues are shown as khz

Edited by littlesat, 11 October 2016 - 17:41.

[Dimitrij]

Need adapt "Cable Scan" plugin

+

maybe this

-		self.list.append(getConfigListEntry(_('Network ID'), config.plugins.CableScan.networkid))
+		self.list.append(getConfigListEntry(_('Network ID') + _(' (0 - all network)'), config.plugins.CableScan.networkid))

[Huevos]

So all these code changes and all you have gained is a dot in the user interface. Apart from that both are six digits and work identically.

[littlesat]

Yes it is... But with dot it is in MHz and without in KHz... It is only UI that is as it should be... Read back the discussion... Sounds stupid indeed when you only look in code and forget about the complete overview...

And the biggest pain was that is was not made the right way from the beginning...

And probably the config float class could be slightly improved to make it at higher level a bit more readable... (E.g. Use the getfloat function and add a set float function, extra conversion, but makes it more readable... Or ad a valueInt property, to give the value times 1000 is integer)

Edited by littlesat, 11 October 2016 - 21:23.

[WanWizard]

I think the remark is that the entire code could have been in kHz, in an integer, and just add a dot in the middle for display purposes.

 

That perhaps would have been simpler, but would require entry in kHz, always, so entering 6 instead of 3 digits for most people. Alternative would be to enter in Mhz as a float, and then directly convert it to integer after input. But that makes the input different from the processing, and isn't exactly clear either.

 

I think I agree with littlesat that although this is more work, it gives a cleaner end result.

[littlesat]

And still one bug to go... The 000.466 bug

[Huevos]

And still one bug to go... The 000.466 bug

I already solved that

[Huevos]

I think the remark is that the entire code could have been in kHz, in an integer, and just add a dot in the middle for display purposes.

 

That perhaps would have been simpler, but would require entry in kHz, always, so entering 6 instead of 3 digits for most people.

There is no difference. The display is 6 figures for both. If you don't want to use the last three figure just leave them as zeros. That it the same for both. If the 6 figures are prefilled that is the same for both. Only difference is the UI shows a decimal point and the code is unnecessarily more complicated.

 

And configFloat isn't even a float, it is 2 integers side by side. The whole thing is completely crazy.

[athoik]

I would prefer the ConfigFloat to simply format number as MHz and keep internally kHz in order not to mess with lists.

BTW why not?

class ConfigFloat(ConfigSequence):
	def __init__(self, default, limits):
		ConfigSequence.__init__(self, seperator = ".", limits = limits, default = default)

	def getFloat(self):
		return float(self.value[1] / float(self.limits[1][1] + 1) + self.value[0])

	def getkHz(self):
		return self.value[0] * 1000 + self.value[1]

	float = property(getFloat)
	kHz = property(getkHz)

Then:

-compare = [1, self.scan_cab.frequency.value[0]*1000 + self.scan_cab.frequency.value[1], self.scan_cab.symbolrate.value*1000, self.scan_cab.modulation.value, self.scan_cab.fec.value, self.scan_cab.inversion.value, self.scan_cab.system.value]
+compare = [1, self.scan_cab.frequency.kHz, self.scan_cab.symbolrate.value*1000, self.scan_cab.modulation.value, self.scan_cab.fec.value, self.scan_cab.inversion.value, self.scan_cab.system.value]

Now isn't more clear?

[WanWizard]

 

I think the remark is that the entire code could have been in kHz, in an integer, and just add a dot in the middle for display purposes.

 

That perhaps would have been simpler, but would require entry in kHz, always, so entering 6 instead of 3 digits for most people.

 

There is no difference. The display is 6 figures for both. If you don't want to use the last three figure just leave them as zeros. That it the same for both. If the 6 figures are prefilled that is the same for both. Only difference is the UI shows a decimal point and the code is unnecessarily more complicated.

 

And configFloat isn't even a float, it is 2 integers side by side. The whole thing is completely crazy.

 

Hmm... If you put it that way...

[WanWizard]

@Athiok,

 

I would not do that, kHz has nothing to do with ConfigFloat, so that method doesn't belong there. Then it's better to wrap or extend ConfigFloat, or make a Frequency class.

[athoik]

I guess you get the point, better to make another class, rather than make code unreadable.

PS. Also frequency has nothing to do with Float, is an unsigned integer not a float.

Edited by athoik, 11 October 2016 - 22:14.

[littlesat]

I just added something general in the ConfigFloat class...

 

In this clase you always define the number of digits behind the dot... now I added a property floatint (with setfloatint and getfloatint) that returns an integer of the value times the digits you get...

 

So basically it returns KHz.... ....

 

And you can also set it with this property... in KHz.... (as we have 3 digits behind the dot and fill in it as MHz)...

 

I could not find a better name for the property as floatint.... is indicates it is a float and and integer that keeps all available digits upto what you defined what is behind the dot...

 

It gives also a very simplified patch.... Just define the ConfigInteger by ConfigFloat and replace the value by floatint....

 

I did not decide to change the value property because the ConfigFloat class could be used (guaranteed) on other locations and it might break other locations and plugins....

Edited by littlesat, 11 October 2016 - 22:27.

[athoik]

Please see WanWizard comments.

Lets go for a frequency class that formats the frequency based on a skin parameter.

Please lets fix it properly.

[littlesat]

Then it's better to wrap or extend ConfigFloat, or make a Frequency class.

 

 

 

Lets go for a frequency class that formats the frequency based on a skin parameter.

I did extend the ConfigFloat class... I do not think we need to add a special config class for the DVB-C/T/ATSC frequencies as the float class covers "all"...

Edited by littlesat, 11 October 2016 - 23:15.

[littlesat]

For now I want to sleep on it......

Edited by littlesat, 11 October 2016 - 23:20.

[Huevos]

All this stress just to display an unnecessary dot in the user interface.

[littlesat]

Yep.... sometimes live is not easy....

I'm still convinced the "." is mandatory....

 

not for entering the frequency... but having it as it should always be....

Edited by littlesat, 12 October 2016 - 15:27.