search

EnviroDIY / YosemitechModbus

A library to use an Arduino as a master to control and communicate with the modbus sensors produced by Yosemitech. Depends on the EnviroDIY/SensorModbusMaster library.
Other
10 stars 7 forks source link

Restore backward compatibility with Y550 #21

Closed aufdenkampe closed 1 year ago

aufdenkampe commented 2 years ago

As @neilh10 points out in https://github.com/EnviroDIY/YosemitechModbus/commit/416fa28d425bfc625cd870844f22ab893da217bd#commitcomment-65821601, I should not have removed backward compatibility with the Y550 sensor when I implemented capabilities for the Y551 COD sensor with:

...and released as v0.3.0.

He says:

An FYI observation - for an .h its often useful to mark something as superseded than remove it. I've just spent 2hrs chasing down a compile issue for src/YosemitechModbus.h where the Y550 was removed and causing the tip version of src/YosemitechModbus.cpp to fail. The solution is to reference for it to work https://github.com/EnviroDIY/YosemitechModbus#v0.2.5

@neilh10, It's clear that I messed that up. I'm really sorry about that. I really didn't think anyone had ever used the Y550, but I see that my shortcut was a mistake.

aufdenkampe commented 2 years ago

@neilh10, I just issued commit 795c1b33f9db70aaba1acd078f85916a6e946ee4 that fix your build issue:

Could you try your build with this reference: https://github.com/EnviroDIY/YosemitechModbus.git#795c1b33f9db70aaba1acd078f85916a6e946ee4

If it works, I'll issue a bug fix release.

neilh10 commented 2 years ago

Great that works for my build. I think the issue is that it is referenced in YosemitechParent.cpp - 

// Needed for newer sensors that do not immediate activate on getting power
    if (_model == Y511 || _model == Y514 || _model == Y550 || _model == Y4000) {

and the builds (my development build at least) pulled in https://github.com/EnviroDIY/YosemitechModbus latest unless explicitly defined to pull in a specific release So that then caused a cc issue.

I think the tip (master) is more defined about which builds to pull in. So I'm not using YosemitechParent.cpp, but for any build to complete it needs to cc :( Much appreciate the update.

aufdenkampe commented 2 years ago

@neilh10, thanks for that feedback. I just merged my fix in to Master with PR#22, for anyone who just points their build to the latest on the master branch.

As you mention, recent versions of ModularSensors now specify exact versions of dependencies, so anyone compiling against the current release of ModularSensors would still get the previous release v0.2.5 of YosemitechModbus.

I'll issue a bug fix release v0.3.1 of YosemitechModbus after I address that dependency tree in ModularSensors.

neilh10 commented 2 years ago

Hey thanks - it works for me from https://github.com/EnviroDIY/YosemitechModbus

aufdenkampe commented 1 year ago

This was fixed in Feb.