Modbus protocol - removeTask

Hi all

I am struggling with an issue on removing tasks from a modbus protocol. I can add a task to extend the protocol - but so far not successful in removing the task again.

I have another function that passes lowNoiseModeState true/false. I works, every bit of the code is executed without faults. Even in the modbus bridge everything is passed correctly.

When I activate (add) to the protocol the number of tasks iincrease from 9 to 11 as expected - but when removing the tasks the number of tasks remains at 11?

What am I doing wrong? Is it wrong at all - or am I just misreading the numbers?

	private synchronized void handleLowNoiseModeProtocol(boolean lowNoiseModeState) throws OpenemsException {
		System.out.println("Number of tasks pre handling: " +  this.getModbusProtocol().getTaskManager().countTasks());
		System.out.println(lowNoiseModeState);
		
		final Task WriteTask = new FC16WriteRegistersTask(111,
				m(PmhModbus.ChannelId.LOW_NOISE_DELTA_T, new UnsignedWordElement(111)), // 0x006F
				m(PmhModbus.ChannelId.LOW_NOISE_TIMER_SUNDAY, new UnsignedWordElement(112)), // 0x0070
				m(PmhModbus.ChannelId.LOW_NOISE_TIMER_MONDAY, new UnsignedWordElement(113)), // 0x0071
				m(PmhModbus.ChannelId.LOW_NOISE_TIMER_TUESDAY, new UnsignedWordElement(114)), // 0x0072
				m(PmhModbus.ChannelId.LOW_NOISE_TIMER_WEDNESDAY, new UnsignedWordElement(115)), // 0x0073
				m(PmhModbus.ChannelId.LOW_NOISE_TIMER_THURSDAY, new UnsignedWordElement(116)), // 0x0074
				m(PmhModbus.ChannelId.LOW_NOISE_TIMER_FRIDAY, new UnsignedWordElement(117)), // 0x0075
				m(PmhModbus.ChannelId.LOW_NOISE_TIMER_SATURDAY, new UnsignedWordElement(118)) // 0x0076
				);
		
		final Task ReadTask = new FC3ReadRegistersTask(111, Priority.HIGH,
				m(PmhModbus.ChannelId.LOW_NOISE_DELTA_T, new UnsignedWordElement(111)), // 0x006F
				m(PmhModbus.ChannelId.LOW_NOISE_TIMER_SUNDAY, new UnsignedWordElement(112)), // 0x0070
				m(PmhModbus.ChannelId.LOW_NOISE_TIMER_MONDAY, new UnsignedWordElement(113)), // 0x0071
				m(PmhModbus.ChannelId.LOW_NOISE_TIMER_TUESDAY, new UnsignedWordElement(114)), // 0x0072
				m(PmhModbus.ChannelId.LOW_NOISE_TIMER_WEDNESDAY, new UnsignedWordElement(115)), // 0x0073
				m(PmhModbus.ChannelId.LOW_NOISE_TIMER_THURSDAY, new UnsignedWordElement(116)), // 0x0074
				m(PmhModbus.ChannelId.LOW_NOISE_TIMER_FRIDAY, new UnsignedWordElement(117)), // 0x0075
				m(PmhModbus.ChannelId.LOW_NOISE_TIMER_SATURDAY, new UnsignedWordElement(118)) // 0x0076
				);
		
		
		if (lowNoiseModeState) {
			try {
				this.getModbusProtocol().addTask(ReadTask);
				this.getModbusProtocol().addTask(WriteTask);
				} catch (OpenemsException e) {
					this.logError(this.log,  "Unable to add low noise mode to protocol: " + e.getMessage());
				} 
		 } else if (!lowNoiseModeState) {
			try {
				this.getModbusProtocol().removeTask(ReadTask);
				this.getModbusProtocol().removeTask(WriteTask);
				} catch (OpenemsException e) {
					this.logError(this.log,  "Unable to remove low noise mode to protocol: " + e.getMessage());
				}
		 }
		
		System.out.println("Number of tasks post handling: " +  this.getModbusProtocol().getTaskManager().countTasks());
		System.out.println("Modbus debuglog " + this.getBridgeModbus().debugLog());
	};

Please test this:

private synchronized void handleLowNoiseModeProtocol(boolean lowNoiseModeState) throws OpenemsException {
    System.out.println("Number of tasks pre handling: " + this.getModbusProtocol().getTaskManager().countTasks());
    System.out.println(lowNoiseModeState);

    final Task WriteTask = new FC16WriteRegistersTask(111,
            m(PmhModbus.ChannelId.LOW_NOISE_DELTA_T, new UnsignedWordElement(111)), // 0x006F
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_SUNDAY, new UnsignedWordElement(112)), // 0x0070
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_MONDAY, new UnsignedWordElement(113)), // 0x0071
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_TUESDAY, new UnsignedWordElement(114)), // 0x0072
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_WEDNESDAY, new UnsignedWordElement(115)), // 0x0073
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_THURSDAY, new UnsignedWordElement(116)), // 0x0074
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_FRIDAY, new UnsignedWordElement(117)), // 0x0075
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_SATURDAY, new UnsignedWordElement(118)) // 0x0076
    );

    final Task ReadTask = new FC3ReadRegistersTask(111, Priority.HIGH,
            m(PmhModbus.ChannelId.LOW_NOISE_DELTA_T, new UnsignedWordElement(111)), // 0x006F
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_SUNDAY, new UnsignedWordElement(112)), // 0x0070
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_MONDAY, new UnsignedWordElement(113)), // 0x0071
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_TUESDAY, new UnsignedWordElement(114)), // 0x0072
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_WEDNESDAY, new UnsignedWordElement(115)), // 0x0073
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_THURSDAY, new UnsignedWordElement(116)), // 0x0074
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_FRIDAY, new UnsignedWordElement(117)), // 0x0075
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_SATURDAY, new UnsignedWordElement(118)) // 0x0076
    );

    if (lowNoiseModeState) {
        try {
            this.getModbusProtocol().addTask(ReadTask);
            this.getModbusProtocol().addTask(WriteTask);
        } catch (OpenemsException e) {
            this.logError(this.log, "Unable to add low noise mode to protocol: " + e.getMessage());
        }
    } else if (!lowNoiseModeState) {
        try {
            boolean removedReadTask = this.getModbusProtocol().removeTask(ReadTask);
            boolean removedWriteTask = this.getModbusProtocol().removeTask(WriteTask);
            
            System.out.println("Removed ReadTask: " + removedReadTask);
            System.out.println("Removed WriteTask: " + removedWriteTask);

            System.out.println("Tasks after attempting to remove:");
            for (Task task : this.getModbusProtocol().getTaskManager().getAllTasks()) {
                System.out.println(task);
            }
            
        } catch (OpenemsException e) {
            this.logError(this.log, "Unable to remove low noise mode to protocol: " + e.getMessage());
        }
    }

    System.out.println("Number of tasks post handling: " + this.getModbusProtocol().getTaskManager().countTasks());
    System.out.println("Modbus debuglog " + this.getBridgeModbus().debugLog());
};

Thank you for your feedback

I had to do some small modifications. For one .removeTask is defined as void - I changed it to get a boolean return:

	/**
	 * Removes a Task from the Protocol.
	 *
	 * @param task the task
	 */
	//public synchronized void removeTask(Task task) {
	//	this.taskManager.removeTask(task);
	//}
	public synchronized boolean removeTask(Task task) {
		try {
			this.taskManager.removeTask(task);
			return true;
		} finally {
		}
	}

From the console I am adding these two taks:

Adding task: io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@e96b46b
Adding task: io.openems.edge.bridge.modbus.api.task.FC16WriteRegistersTask@1d7304e6

Calling the removeTask I get “true” as returned value

Number of taks are the same and the list of taks still contains the above two task:

Removed ReadTask: true
Removed WriteTask: true
Tasks after attempting to remove:
io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@156aa4e2
io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@59740c6e
io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@62fadec7
io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@672ba24a
io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@272657f9
io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@5ec8380f
io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@6af0c9f7
io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@50f31f11
io.openems.edge.bridge.modbus.api.task.FC16WriteRegistersTask@457f0369
io.openems.edge.bridge.modbus.api.task.FC16WriteRegistersTask@5ae85389
io.openems.edge.bridge.modbus.api.task.FC16WriteRegistersTask@363b9652
io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@e96b46b
io.openems.edge.bridge.modbus.api.task.FC16WriteRegistersTask@1d7304e6

I am currently not connected to the Modbus Slave - hence the timouts caused by improper reply are triggered. I have access to the slave on thursday and plan will test if it makes a difference. I have not traced all the details of task handling - but maybe task cannot be removed if they are in “timeout quarantine”? Just guessing.

I have tried to scan through the modbus bridge test files - but did not find any usage of removeTask.

I really have no clue why the removeTask is executed without result.

Are you sure the removed task does not disappear after a while - i.e. one minute?

We recently found a little bug, that would cause an internal queue to still hold a removed task. It’s fixed by adjusting the code as follows. In rest removing works fine for us.

public synchronized void removeProtocol(String sourceId) {
	this.taskManagers.remove(sourceId);
	this.nextLowPriorityTasks.removeIf(t -> t.a() == sourceId);
}

→ openems/io.openems.edge.bridge.modbus/src/io/openems/edge/bridge/modbus/api/worker/internal/TasksSupplierImpl.java at develop · OpenEMS/openems · GitHub

1 Like

It seems like the task instances being removed are not the same as those added, which can be due to several reasons such as task identity mismatch. Ensure that the exact same task instances are being removed, or alternatively, use task identifiers to remove the correct tasks. Also, ensure that your task manager’s removeTask method correctly identifies and removes tasks.

Here’s a suggested approach to debug and resolve the issue:

  1. Use Identifiers for Tasks: Ensure tasks are uniquely identifiable, and remove them based on identifiers.
  2. Check Task Instance Equality: Verify that the tasks being removed are the same instances that were added.
  3. Debug Task Removal: Add debug statements to ensure that the tasks you are trying to remove exist in the task manager.

Updated Code

Modify your removeTask method to log more information and ensure tasks are correctly identified:

private synchronized void handleLowNoiseModeProtocol(boolean lowNoiseModeState) throws OpenemsException {
    System.out.println("Number of tasks pre handling: " + this.getModbusProtocol().getTaskManager().countTasks());
    System.out.println("Low Noise Mode State: " + lowNoiseModeState);

    final Task writeTask = new FC16WriteRegistersTask(111,
            m(PmhModbus.ChannelId.LOW_NOISE_DELTA_T, new UnsignedWordElement(111)),
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_SUNDAY, new UnsignedWordElement(112)),
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_MONDAY, new UnsignedWordElement(113)),
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_TUESDAY, new UnsignedWordElement(114)),
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_WEDNESDAY, new UnsignedWordElement(115)),
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_THURSDAY, new UnsignedWordElement(116)),
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_FRIDAY, new UnsignedWordElement(117)),
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_SATURDAY, new UnsignedWordElement(118))
    );

    final Task readTask = new FC3ReadRegistersTask(111, Priority.HIGH,
            m(PmhModbus.ChannelId.LOW_NOISE_DELTA_T, new UnsignedWordElement(111)),
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_SUNDAY, new UnsignedWordElement(112)),
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_MONDAY, new UnsignedWordElement(113)),
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_TUESDAY, new UnsignedWordElement(114)),
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_WEDNESDAY, new UnsignedWordElement(115)),
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_THURSDAY, new UnsignedWordElement(116)),
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_FRIDAY, new UnsignedWordElement(117)),
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_SATURDAY, new UnsignedWordElement(118))
    );

    if (lowNoiseModeState) {
        try {
            this.getModbusProtocol().addTask(readTask);
            this.getModbusProtocol().addTask(writeTask);
        } catch (OpenemsException e) {
            this.logError(this.log, "Unable to add low noise mode to protocol: " + e.getMessage());
        }
    } else {
        try {
            boolean removedReadTask = this.getModbusProtocol().removeTask(readTask);
            boolean removedWriteTask = this.getModbusProtocol().removeTask(writeTask);

            System.out.println("Removed ReadTask: " + removedReadTask);
            System.out.println("Removed WriteTask: " + removedWriteTask);

            // Debugging: print remaining tasks
            System.out.println("Tasks after attempting to remove:");
            for (Task task : this.getModbusProtocol().getTaskManager().getAllTasks()) {
                System.out.println(task);
            }

        } catch (OpenemsException e) {
            this.logError(this.log, "Unable to remove low noise mode from protocol: " + e.getMessage());
        }
    }

    System.out.println("Number of tasks post handling: " + this.getModbusProtocol().getTaskManager().countTasks());
    System.out.println("Modbus debuglog: " + this.getBridgeModbus().debugLog());
}

Key Changes:

  • Boolean Return for removeTask: Ensure removeTask returns a boolean to confirm task removal.
  • Debugging Statements: Added detailed logging for task removal and the current state of tasks.

TaskManager’s removeTask Method:

Make sure removeTask correctly removes tasks and returns a boolean:

public synchronized boolean removeTask(Task task) {
    return this.taskManager.removeTask(task);
}

Conclusion

If the issue persists, it could be due to how tasks are stored or identified within the TaskManager. Double-check the implementation of TaskManager to ensure tasks are uniquely identified and correctly managed.

1 Like

You are right, the tasks attempted to be removed are different from the ones being added. In fact, they do not exist on the list of tasks.

A hint for using the identifier approach would be appreciated. Thank you.

Number of tasks pre handling: 11
true
Tasks before adding:
io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@529afd71
io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@2e65cd92
io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@7a6cff73
io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@5381b8ed
io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@5675a699
io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@56fcf898
io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@4bf61253
io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@156e8d10
io.openems.edge.bridge.modbus.api.task.FC16WriteRegistersTask@6ed4b04a
io.openems.edge.bridge.modbus.api.task.FC16WriteRegistersTask@5fddbd30
io.openems.edge.bridge.modbus.api.task.FC16WriteRegistersTask@24425812
Adding task: io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@45a54787
Adding task: io.openems.edge.bridge.modbus.api.task.FC16WriteRegistersTask@e38f999
Number of tasks post handling: 13
Modbus debuglog null
2024-05-15T19:54:50,147 [_cycle  ] INFO  [ontroller.api.common.ApiWorker] [ctrlApiWebsocket0] Set Channel [ecp.ems.edge.hp.pmh0/LowNoiseEnable] to Value [1]
2024-05-15T19:54:50,846 [et Api-2] INFO  [ontroller.api.common.ApiWorker] [ctrlApiWebsocket0] Set [ecp.ems.edge.hp.pmh0/LowNoiseEnable] to [0] via API. Timeout is [60s]
2024-05-15T19:54:51,143 [_cycle  ] INFO  [ontroller.api.common.ApiWorker] [ctrlApiWebsocket0] Set Channel [ecp.ems.edge.hp.pmh0/LowNoiseEnable] to Value [0]
Number of tasks pre handling: 13
false
Removing task: io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@3cbee153
Removing task: io.openems.edge.bridge.modbus.api.task.FC16WriteRegistersTask@a8aac1e
Removed ReadTask: true
Removed WriteTask: true
Tasks after attempting to remove:
io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@529afd71
io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@2e65cd92
io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@7a6cff73
io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@5381b8ed
io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@5675a699
io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@56fcf898
io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@4bf61253
io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@156e8d10
io.openems.edge.bridge.modbus.api.task.FC16WriteRegistersTask@6ed4b04a
io.openems.edge.bridge.modbus.api.task.FC16WriteRegistersTask@5fddbd30
io.openems.edge.bridge.modbus.api.task.FC16WriteRegistersTask@24425812
io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@45a54787
io.openems.edge.bridge.modbus.api.task.FC16WriteRegistersTask@e38f999
Number of tasks post handling: 13
Modbus debuglog null
  1. Add Identifiers to Tasks: Modify the Task class to include a unique identifier.
  2. Use Identifiers to Manage Tasks: Add and remove tasks based on their identifiers.

First, modify the Task class (or the classes that extend Task) to include an identifier:

public abstract class Task {
    private final String id;

    public Task(String id) {
        this.id = id;
    }

    public String getId() {
        return id;
    }

    @Override
    public boolean equals(Object obj) {
        if (this == obj) return true;
        if (obj == null || getClass() != obj.getClass()) return false;
        Task task = (Task) obj;
        return id.equals(task.id);
    }

    @Override
    public int hashCode() {
        return Objects.hash(id);
    }
}

Now, update your tasks to include a unique identifier:

private synchronized void handleLowNoiseModeProtocol(boolean lowNoiseModeState) throws OpenemsException {
    System.out.println("Number of tasks pre handling: " + this.getModbusProtocol().getTaskManager().countTasks());
    System.out.println("Low Noise Mode State: " + lowNoiseModeState);

    final String writeTaskId = "writeTask";
    final String readTaskId = "readTask";

    final Task writeTask = new FC16WriteRegistersTask(writeTaskId, 111,
            m(PmhModbus.ChannelId.LOW_NOISE_DELTA_T, new UnsignedWordElement(111)),
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_SUNDAY, new UnsignedWordElement(112)),
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_MONDAY, new UnsignedWordElement(113)),
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_TUESDAY, new UnsignedWordElement(114)),
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_WEDNESDAY, new UnsignedWordElement(115)),
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_THURSDAY, new UnsignedWordElement(116)),
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_FRIDAY, new UnsignedWordElement(117)),
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_SATURDAY, new UnsignedWordElement(118))
    );

    final Task readTask = new FC3ReadRegistersTask(readTaskId, 111, Priority.HIGH,
            m(PmhModbus.ChannelId.LOW_NOISE_DELTA_T, new UnsignedWordElement(111)),
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_SUNDAY, new UnsignedWordElement(112)),
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_MONDAY, new UnsignedWordElement(113)),
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_TUESDAY, new UnsignedWordElement(114)),
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_WEDNESDAY, new UnsignedWordElement(115)),
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_THURSDAY, new UnsignedWordElement(116)),
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_FRIDAY, new UnsignedWordElement(117)),
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_SATURDAY, new UnsignedWordElement(118))
    );

    if (lowNoiseModeState) {
        try {
            this.getModbusProtocol().addTask(readTask);
            this.getModbusProtocol().addTask(writeTask);
        } catch (OpenemsException e) {
            this.logError(this.log, "Unable to add low noise mode to protocol: " + e.getMessage());
        }
    } else {
        try {
            boolean removedReadTask = this.getModbusProtocol().removeTaskById(readTaskId);
            boolean removedWriteTask = this.getModbusProtocol().removeTaskById(writeTaskId);

            System.out.println("Removed ReadTask: " + removedReadTask);
            System.out.println("Removed WriteTask: " + removedWriteTask);

            // Debugging: print remaining tasks
            System.out.println("Tasks after attempting to remove:");
            for (Task task : this.getModbusProtocol().getTaskManager().getAllTasks()) {
                System.out.println(task);
            }

        } catch (OpenemsException e) {
            this.logError(this.log, "Unable to remove low noise mode from protocol: " + e.getMessage());
        }
    }

    System.out.println("Number of tasks post handling: " + this.getModbusProtocol().getTaskManager().countTasks());
    System.out.println("Modbus debuglog: " + this.getBridgeModbus().debugLog());
}

Updated TaskManager:

Ensure your TaskManager can handle tasks by identifier:

public class TaskManager {
    private Map<String, Task> tasks = new ConcurrentHashMap<>();

    public void addTask(Task task) {
        tasks.put(task.getId(), task);
    }

    public boolean removeTaskById(String taskId) {
        return tasks.remove(taskId) != null;
    }

    public Collection<Task> getAllTasks() {
        return tasks.values();
    }

    public int countTasks() {
        return tasks.size();
    }
}

Moin Stefan

Thank you for your input. I expected this to be part of the issue, but as @Sn0w3y guided me to discover, the task reference seems to change in my implementation between adding and removing tasks.

On my wish list would be some sort of complaining from the removeTask method if it is asked to remove a non-existing task. I will see if I can figure out to add this at some point.

Thank you @Sn0w3y for the detailed reply.

Your guidance helped a lot in discovering the change of task reference between adding and my attempt to remove the task.

I will consider to implement your concept of identifiers associated with the task - for now I have moved the declarations of the tasks to be added/removed outside the method. Then they are only invoked once and do not seem to change.

The tasks are defined:

public class PmhModbusImpl extends AbstractOpenemsModbusComponent implements PmhModbus, ModbusComponent, OpenemsComponent {

	@Reference
	private ConfigurationAdmin cm;
	
	private final Logger log = LoggerFactory.getLogger(PmhModbusImpl.class);
	
	private final Task lowNoiseWriteTask = new FC16WriteRegistersTask(111,
			m(PmhModbus.ChannelId.LOW_NOISE_DELTA_T, new UnsignedWordElement(111)), // 0x006F
	        m(PmhModbus.ChannelId.LOW_NOISE_TIMER_SUNDAY, new UnsignedWordElement(112)), // 0x0070
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_MONDAY, new UnsignedWordElement(113)), // 0x0071
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_TUESDAY, new UnsignedWordElement(114)), // 0x0072
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_WEDNESDAY, new UnsignedWordElement(115)), // 0x0073
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_THURSDAY, new UnsignedWordElement(116)), // 0x0074
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_FRIDAY, new UnsignedWordElement(117)), // 0x0075
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_SATURDAY, new UnsignedWordElement(118)) // 0x0076
    );

	private final Task lowNoiseReadTask = new FC3ReadRegistersTask(111, Priority.LOW,
            m(PmhModbus.ChannelId.LOW_NOISE_DELTA_T, new UnsignedWordElement(111)), // 0x006F
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_SUNDAY, new UnsignedWordElement(112)), // 0x0070
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_MONDAY, new UnsignedWordElement(113)), // 0x0071
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_TUESDAY, new UnsignedWordElement(114)), // 0x0072
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_WEDNESDAY, new UnsignedWordElement(115)), // 0x0073
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_THURSDAY, new UnsignedWordElement(116)), // 0x0074
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_FRIDAY, new UnsignedWordElement(117)), // 0x0075
            m(PmhModbus.ChannelId.LOW_NOISE_TIMER_SATURDAY, new UnsignedWordElement(118)) // 0x0076
    );

And the method changed to:

	private synchronized void handleLowNoiseModeProtocol(boolean lowNoiseModeState) throws OpenemsException {
	    System.out.println("Number of tasks pre handling: " + this.getModbusProtocol().getTaskManager().countTasks());
	    System.out.println(lowNoiseModeState);
	    
	    System.out.println("readTask before if statement: " + this.lowNoiseReadTask.toString());

	    if (lowNoiseModeState) {
	        try {
	        	System.out.println("Tasks before adding:");
	            for (Task task : this.getModbusProtocol().getTaskManager().getTasks()) {
	                System.out.println(task);
	            }
	        	
	        	
	        	System.out.println("Adding task: " + this.lowNoiseReadTask);
	        	System.out.println("Adding task: " + this.lowNoiseWriteTask);
	        		        	        	
	            this.getModbusProtocol().addTask(this.lowNoiseReadTask);
	            this.getModbusProtocol().addTask(this.lowNoiseWriteTask);
	            
	            
	            System.out.println("Tasks after adding:");
	            for (Task task : this.getModbusProtocol().getTaskManager().getTasks()) {
	                System.out.println(task);
	            }
            
	        } catch (OpenemsException e) {
	            this.logError(this.log, "Unable to add low noise mode to protocol: " + e.getMessage());
	        }
	    } else if (!lowNoiseModeState) {
	        try {
	        	System.out.println("Removing task: " + this.lowNoiseReadTask);
	        	System.out.println("Removing task: " + this.lowNoiseWriteTask);
	            boolean removedReadTask = this.getModbusProtocol().removeTask(this.lowNoiseReadTask);
	            boolean removedWriteTask = this.getModbusProtocol().removeTask(this.lowNoiseWriteTask);
	            
	            System.out.println("Removed ReadTask: " + removedReadTask);
	            System.out.println("Removed WriteTask: " + removedWriteTask);
	            
	            System.out.println("Tasks after attempting to remove:");
	            for (Task task : this.getModbusProtocol().getTaskManager().getTasks()) {
	                System.out.println(task);
	            }
	            
	        } catch (OpenemsException e) {
	            this.logError(this.log, "Unable to remove low noise mode to protocol: " + e.getMessage());
	        }
	    }

	    System.out.println("Number of tasks post handling: " + this.getModbusProtocol().getTaskManager().countTasks());
	    System.out.println("Modbus debuglog " + this.getBridgeModbus().debugLog());
	};

Which is now successful:

Number of tasks pre handling: 11
true
readTask before if statement: io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@4ddbc111
Tasks before adding:
io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@29647edc
io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@7f0ef362
io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@41f2c9bc
io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@52b24bda
io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@582a4dce
io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@50b7304a
io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@75facc9
io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@4773db63
io.openems.edge.bridge.modbus.api.task.FC16WriteRegistersTask@4d3dba7e
io.openems.edge.bridge.modbus.api.task.FC16WriteRegistersTask@24e1f6f3
io.openems.edge.bridge.modbus.api.task.FC16WriteRegistersTask@5e222f42
Adding task: io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@4ddbc111
Adding task: io.openems.edge.bridge.modbus.api.task.FC16WriteRegistersTask@7d35f26
Number of tasks post handling: 13
Modbus debuglog null
2024-05-16T08:37:25,954 [_cycle  ] INFO  [ontroller.api.common.ApiWorker] [ctrlApiWebsocket0] Set Channel [ecp.ems.edge.hp.pmh0/LowNoiseEnable] to Value [1]
2024-05-16T08:37:26,648 [et Api-7] INFO  [ontroller.api.common.ApiWorker] [ctrlApiWebsocket0] Set [ecp.ems.edge.hp.pmh0/LowNoiseEnable] to [0] via API. Timeout is [60s]
2024-05-16T08:37:26,950 [_cycle  ] INFO  [ontroller.api.common.ApiWorker] [ctrlApiWebsocket0] Set Channel [ecp.ems.edge.hp.pmh0/LowNoiseEnable] to Value [0]
Number of tasks pre handling: 13
false
readTask before if statement: io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@4ddbc111
Removing task: io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@4ddbc111
Removing task: io.openems.edge.bridge.modbus.api.task.FC16WriteRegistersTask@7d35f26
Removed ReadTask: true
Removed WriteTask: true
Tasks after attempting to remove:
io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@29647edc
io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@7f0ef362
io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@41f2c9bc
io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@52b24bda
io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@582a4dce
io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@50b7304a
io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@75facc9
io.openems.edge.bridge.modbus.api.task.FC3ReadRegistersTask@4773db63
io.openems.edge.bridge.modbus.api.task.FC16WriteRegistersTask@4d3dba7e
io.openems.edge.bridge.modbus.api.task.FC16WriteRegistersTask@24e1f6f3
io.openems.edge.bridge.modbus.api.task.FC16WriteRegistersTask@5e222f42
Number of tasks post handling: 11
Modbus debuglog null
1 Like

One final note. When the task is removed I set the channel next value to NULL to clear the last read value. Just to avoid confusion on channels being populated with values even when not being read.

1 Like