Skip to content

TLA2528 - #5618

Open
SuksAE wants to merge 11 commits into
letscontrolit:megafrom
SuksAE:TLA2528
Open

TLA2528#5618
SuksAE wants to merge 11 commits into
letscontrolit:megafrom
SuksAE:TLA2528

Conversation

@SuksAE

@SuksAE SuksAE commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Here is the pull request for the TLA2528 Plugin...

Thanks

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do not commit this file, it is included in .gitignore, but unfortunately not really ignored 😒

Comment thread docs/source/Plugin/P188.rst Outdated
Comment on lines +130 to +131
|added|
Initial release version.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A date is missing here?

Comment on lines +111 to +114
#define P188_CAL_RAW2_P1 PCONFIG_LONG(4)
#define P188_CAL_RAW2_P2 PCONFIG_LONG(5)
#define P188_CAL_RAW3_P1 PCONFIG_LONG(6)
#define P188_CAL_RAW3_P2 PCONFIG_LONG(7)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are only 4 PCONFIG_LONG available. This will overwrite the values for the next task (or be overwritten if the next task is saved). And it can't be extended as that will render all existing settings files incompatible.

Comment on lines +120 to +123
#define P188_CAL_OUT2_P1 PCONFIG_FLOAT(4)
#define P188_CAL_OUT2_P2 PCONFIG_FLOAT(5)
#define P188_CAL_OUT3_P1 PCONFIG_FLOAT(6)
#define P188_CAL_OUT3_P2 PCONFIG_FLOAT(7)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto for PCONFIG_FLOAT.

Comment thread src/_P188_TLA2528.ino Outdated
Comment on lines +32 to +46
dev.Ports = 0; // Port to use when device has multiple I/O pins (N.B. not used much)
dev.ValueCount = 4; // The number of output values of a plugin. The value should match the number of keys PLUGIN_VALUENAME1_xxx
dev.OutputDataType = Output_Data_type_t::Simple; // Subset of selectable output data types (Default = no selection)
dev.PullUpOption = false; // Allow to set internal pull-up resistors.
dev.InverseLogicOption = false; // Allow to invert the boolean state (e.g. a switch)
dev.FormulaOption = true; // Allow to enter a formula to convert values during read. (not possible with Custom enabled)
dev.Custom = false;
dev.SendDataOption = true; // Allow to send data to a controller.
dev.GlobalSyncOption = false; // No longer used. Was used for ESPeasy values sync between nodes
dev.TimerOption = true; // Allow to set the "Interval" timer for the plugin.
dev.TimerOptional = false; // When taskdevice timer is not set and not optional, use default "Interval" delay (Settings.Delay)
dev.DecimalsOnly = false; // Allow to set the number of decimals (otherwise treated a 0 decimals)
dev.CustomVTypeVar = true; // Enable to allow the user to configure the Sensor_VType per Value that's available for the plugin
dev.PluginStats = true; // Support for PluginStats to record last N task values, show charts etc.
dev.MqttStateClass = true;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please leave out any = 0 and = false assignment, as that's already taken care of in the constructor of the Device structure.

Comment thread src/_P188_TLA2528.ino
Comment on lines +125 to +127
#ifdef P188_FEATURE_RESISTOR_MEASUREMENT
tmp_config.R_Clip = 0.0f;
#endif

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be solved by an initializer in the struct.

#ifdef P188_FEATURE_RESISTOR_MEASUREMENT
uint32_t Rref[VARS_PER_TASK];
uint32_t Rpar[VARS_PER_TASK];
#endif // P188_FEATURE_RESISTOR_MEASUREMENT

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These variables can be reset initially by using the {} initializer on them, like float R_Clip{};

@SuksAE

SuksAE commented Aug 23, 2026

Copy link
Copy Markdown
Contributor Author

changed code and documentation - please review.
Thanks

Comment thread src/_P188_TLA2528.ino
Comment on lines +155 to +156
if ((P188_OUTPUT_MAPPING_0 > 11) || (P188_OUTPUT_MAPPING_1 > 11) || (P188_OUTPUT_MAPPING_2 > 11) || (P188_OUTPUT_MAPPING_3 > 11))
{

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That magic number 11 should better be a #define or some enum value (also in the other places where it's used), this is very error-prone, and hard to comprehend.

Comment thread src/_P188_TLA2528.ino
addFormCheckBox(F("Enable Calibration"), getPluginCustomArgName(i * 8 + 3), ((P188_configBits.en_cal >> i) & 0x01));

addFormNumericBox(F("Point 1"), getPluginCustomArgName(i * 8 + 4), PCONFIG_LONG(P188_CAL_INDEX + 2*i), -32768, 32767);
addFormFloatNumberBox(F("Point 1"), getPluginCustomArgName(i * 8 + 4), tmp_config.CalIn[i][0], -1000000, 1000000, 2, 1.0f);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The min and max arguments are also floats, that like the .0f suffix.

Comment thread src/_P188_TLA2528.ino
const uint8_t pconfigIndex = P188_OUTPUT_MAPPING_OFFSET + i;
const uint8_t choice = PCONFIG(pconfigIndex);
sensorTypeHelper_saveOutputSelector(event, pconfigIndex, i, Plugin_188_output_mapping_name(choice, false));
if ((P188_OUTPUT_MAPPING_0 > 11) || (P188_OUTPUT_MAPPING_1 > 11) || (P188_OUTPUT_MAPPING_2 > 11) || (P188_OUTPUT_MAPPING_3 > 11))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same magic number

Comment thread src/_P188_TLA2528.ino
#ifdef P188_FEATURE_RESISTOR_MEASUREMENT
addFormNumericBox(F("Reference Resistor Value"), getPluginCustomArgName(i * 8 + 0), tmp_config.Rref[i], 100, 470000);
addFormNumericBox(F("Parallel Resistor Value"), getPluginCustomArgName(i * 8 + 1), tmp_config.Rpar[i], 0, 470000);
if (PCONFIG(i + P188_OUTPUT_MAPPING_OFFSET) > 11)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another one

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also needs another check to make sure the index for PCONFIG(...) is within range.

I will make a separate PR for adding checks for these macros for builds without BUILD_NO_DEBUG enabled, so we can have a runtime check on nodes with a bit more headroom to do some runtime checks to see if there are clear programming errors.

Comment thread src/_P188_TLA2528.ino
#ifdef P188_FEATURE_RESISTOR_MEASUREMENT
tmp_config.Rref[i] = getFormItemInt(getPluginCustomArgName(i * 8 + 0));
tmp_config.Rpar[i] = getFormItemInt(getPluginCustomArgName(i * 8 + 1));
if (PCONFIG(i + P188_OUTPUT_MAPPING_OFFSET) > 11)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another one

Comment thread src/_P188_TLA2528.ino
@@ -312,10 +326,10 @@ boolean Plugin_188(uint8_t function, struct EventStruct *event, String& string)
{
if ((P188_configBits.en_cal >> i) & 0x01)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes this works, but using bitRead() and a properly named bit-index will have more meaning in 6 months...

Comment thread src/_P188_TLA2528.ino
@@ -381,10 +395,10 @@ boolean Plugin_188(uint8_t function, struct EventStruct *event, String& string)

if ((P188_configBits.en_cal >> i) & 0x01)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto

Comment thread src/_P188_TLA2528.ino
const float out1 = P188_data->P188_config.CalOut[i][0];
const float out2 = P188_data->P188_config.CalOut[i][1];

if (adc1 != adc2)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that adc1 and adc2 are float, using !essentiallyEqual() takes care of the inaccuracy of the float type.

Comment thread src/_P188_TLA2528.ino
const float out1 = P188_data->P188_config.CalOut[i][0];
const float out2 = P188_data->P188_config.CalOut[i][1];

if (adc1 != adc2)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto

@tonhuisman

Copy link
Copy Markdown
Contributor

You haven't included the plugin in the PLUGIN_SET_MAX list in define_plugin_sets.h. By default all plugins should at least be included there. I can't yet see in what 'themed' builds it should go, but maybe it should be included in a Collection build (that would be in the Collection H, as the other builds are quite filled already) and for ESP32 only, like the other plugins in there. If you add it there, it should also be reflected in the documentation. The MAX build is never listed there, btw. You have currently listed it for the Normal build, but that's not where it should go (and it isn't in that set either, as you haven't mentioned it in define_plugin_sets.h yet). What goes in Normal is fixed, and decided by TD-er.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants