Per-Device ConfigurationInformation blocks#463
Per-Device ConfigurationInformation blocks#463night199uk wants to merge 1 commit intottlappalainen:masterfrom
Conversation
|
Resolves #461. Victron devices expect InstallationDescription1 to be a per-device attribute containing the device name (CustomName in Victron-land). |
5114267 to
887681b
Compare
887681b to
39c9ac4
Compare
|
@ttlappalainen I removed WIP tag, think this should be pretty complete. Would appreciate feedback. I migrated the ConfigurationInformation to work exactly the same way as ProductInformation w/r/t per-multi-device support. |
|
For compatibility it should work as default as it has been before. |
|
@ttlappalainen Are there specific areas or tests you want to see compatibility with please? I will take a look at the test suite, but didn't see any failures. I don't know all of the use cases the library supports as I've only recently started using it. My basic understanding is that functionally this should work the same as the original ConfigurationInformation implementation, however some of the API functions may not longer be required/supported as they are not needed. |
|
Take first old library version and example TemperatureMonitor. Add to it configuration information and modify it as multidevice. Then try to compile with new version without changing anything on previously modified TemperatureMonitor. Will it compile? Also it should show same configuration information for all devices. |
|
Hi Timon, I took one of my existing projects that already had multi-device support. For testing, I proceeded as follows: I compiled everything using the current main branch — everything worked as before. Then I switched to the PR and recompiled — again, everything behaved as before. The ConfigurationInformation was the same for all devices. Next, I added SetConfigurationInformation with the DeviceIndex and customized Description1 for each device. Each device now had its own description. I also tested SetInstallationDescription1 and SetInstallationDescription2 with the same result. From my perspective, the PR works as expected. I tested this on an ESP32 Node32s and used the NMEAReader for verification. |
|
Just getting back to this as I've been working on other projects. @minou65 thanks for testing! |
|
Checking in here. Is there anything else I need to do to get this landed? This has been working great for me since August monitoring 6 tanks (3 water 3 holding) split 3 each on two HALMETs with SensESP. |
|
Hi @ttlappalainen |
No description provided.